-
Notifications
You must be signed in to change notification settings - Fork 14.9k
[SelectionDAG] Add a flag to sort DAGs before showing them in debug prints #149732
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
@llvm/pr-subscribers-llvm-selectiondag Author: Min-Yih Hsu (mshockwave) ChangesThere are many cases where SDNodes shown in the Full diff: https://github.com/llvm/llvm-project/pull/149732.diff 1 Files Affected:
diff --git a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp
index 26071ed70c9db..4964edd620e47 100644
--- a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp
@@ -144,6 +144,12 @@ UseMBPI("use-mbpi",
cl::init(true), cl::Hidden);
#ifndef NDEBUG
+static cl::opt<bool>
+ SortDAGBeforeDump("sort-dags-before-isel-dump", cl::Hidden,
+ cl::desc("Sort SelectionDAGs before showing them in"
+ " debug prints during SelectionDAGISel."),
+ cl::init(false));
+
static cl::opt<std::string>
FilterDAGBasicBlockName("filter-view-dags", cl::Hidden,
cl::desc("Only display the basic block whose name "
@@ -903,6 +909,14 @@ void SelectionDAGISel::ComputeLiveOutVRegInfo() {
} while (!Worklist.empty());
}
+#ifndef NDEBUG
+static void dumpSelectionDAG(SelectionDAG *DAG) {
+ if (SortDAGBeforeDump)
+ DAG->AssignTopologicalOrder();
+ DAG->dump();
+}
+#endif
+
void SelectionDAGISel::CodeGenAndEmitDAG() {
StringRef GroupName = "sdag";
StringRef GroupDescription = "Instruction Selection and Scheduling";
@@ -930,7 +944,7 @@ void SelectionDAGISel::CodeGenAndEmitDAG() {
ISEL_DUMP(dbgs() << "\nInitial selection DAG: "
<< printMBBReference(*FuncInfo->MBB) << " '" << BlockName
<< "'\n";
- CurDAG->dump());
+ dumpSelectionDAG(CurDAG));
#if !defined(NDEBUG) && LLVM_ENABLE_ABI_BREAKING_CHECKS
if (TTI->hasBranchDivergence())
@@ -950,7 +964,7 @@ void SelectionDAGISel::CodeGenAndEmitDAG() {
ISEL_DUMP(dbgs() << "\nOptimized lowered selection DAG: "
<< printMBBReference(*FuncInfo->MBB) << " '" << BlockName
<< "'\n";
- CurDAG->dump());
+ dumpSelectionDAG(CurDAG));
#if !defined(NDEBUG) && LLVM_ENABLE_ABI_BREAKING_CHECKS
if (TTI->hasBranchDivergence())
@@ -972,7 +986,7 @@ void SelectionDAGISel::CodeGenAndEmitDAG() {
ISEL_DUMP(dbgs() << "\nType-legalized selection DAG: "
<< printMBBReference(*FuncInfo->MBB) << " '" << BlockName
<< "'\n";
- CurDAG->dump());
+ dumpSelectionDAG(CurDAG));
#if !defined(NDEBUG) && LLVM_ENABLE_ABI_BREAKING_CHECKS
if (TTI->hasBranchDivergence())
@@ -996,7 +1010,7 @@ void SelectionDAGISel::CodeGenAndEmitDAG() {
ISEL_DUMP(dbgs() << "\nOptimized type-legalized selection DAG: "
<< printMBBReference(*FuncInfo->MBB) << " '" << BlockName
<< "'\n";
- CurDAG->dump());
+ dumpSelectionDAG(CurDAG));
#if !defined(NDEBUG) && LLVM_ENABLE_ABI_BREAKING_CHECKS
if (TTI->hasBranchDivergence())
@@ -1014,7 +1028,7 @@ void SelectionDAGISel::CodeGenAndEmitDAG() {
ISEL_DUMP(dbgs() << "\nVector-legalized selection DAG: "
<< printMBBReference(*FuncInfo->MBB) << " '" << BlockName
<< "'\n";
- CurDAG->dump());
+ dumpSelectionDAG(CurDAG));
#if !defined(NDEBUG) && LLVM_ENABLE_ABI_BREAKING_CHECKS
if (TTI->hasBranchDivergence())
@@ -1030,7 +1044,7 @@ void SelectionDAGISel::CodeGenAndEmitDAG() {
ISEL_DUMP(dbgs() << "\nVector/type-legalized selection DAG: "
<< printMBBReference(*FuncInfo->MBB) << " '" << BlockName
<< "'\n";
- CurDAG->dump());
+ dumpSelectionDAG(CurDAG));
#if !defined(NDEBUG) && LLVM_ENABLE_ABI_BREAKING_CHECKS
if (TTI->hasBranchDivergence())
@@ -1050,7 +1064,7 @@ void SelectionDAGISel::CodeGenAndEmitDAG() {
ISEL_DUMP(dbgs() << "\nOptimized vector-legalized selection DAG: "
<< printMBBReference(*FuncInfo->MBB) << " '" << BlockName
<< "'\n";
- CurDAG->dump());
+ dumpSelectionDAG(CurDAG));
#if !defined(NDEBUG) && LLVM_ENABLE_ABI_BREAKING_CHECKS
if (TTI->hasBranchDivergence())
@@ -1070,7 +1084,7 @@ void SelectionDAGISel::CodeGenAndEmitDAG() {
ISEL_DUMP(dbgs() << "\nLegalized selection DAG: "
<< printMBBReference(*FuncInfo->MBB) << " '" << BlockName
<< "'\n";
- CurDAG->dump());
+ dumpSelectionDAG(CurDAG));
#if !defined(NDEBUG) && LLVM_ENABLE_ABI_BREAKING_CHECKS
if (TTI->hasBranchDivergence())
@@ -1090,7 +1104,7 @@ void SelectionDAGISel::CodeGenAndEmitDAG() {
ISEL_DUMP(dbgs() << "\nOptimized legalized selection DAG: "
<< printMBBReference(*FuncInfo->MBB) << " '" << BlockName
<< "'\n";
- CurDAG->dump());
+ dumpSelectionDAG(CurDAG));
#if !defined(NDEBUG) && LLVM_ENABLE_ABI_BREAKING_CHECKS
if (TTI->hasBranchDivergence())
@@ -1114,7 +1128,7 @@ void SelectionDAGISel::CodeGenAndEmitDAG() {
ISEL_DUMP(dbgs() << "\nSelected selection DAG: "
<< printMBBReference(*FuncInfo->MBB) << " '" << BlockName
<< "'\n";
- CurDAG->dump());
+ dumpSelectionDAG(CurDAG));
if (ViewSchedDAGs && MatchFilterBB)
CurDAG->viewGraph("scheduler input for " + BlockName);
|
Or we could focus efforts on finally ensuring the DAG is topologically sorted to begin with: #83422 |
True, aside from the problems raised in the said issue, I remember compilation time is/was also a concern? But in any case, I feel like this patch can provide a quick and easy solution for improving developers' life, at least in a short term. |
Agreed, I'm just rather annoyed at the lack on interest in helping with something that should have been done a decade ago..... FTR a topological sorted dag should reduce backend compile time: https://llvm-compile-time-tracker.com/?config=Overview&stat=instructions%3Au&remote=RKSimon |
Close in favor of #161097 |
An alternative approach to #149732 , which sorts the DAG before dumping it. That approach runs a risk of altering the codegen result as we don't know if any of the downstream DAG users relies on the node ID, which was updated as part of the sorting. The new method proposed by this PR does not update the node ID or any of the DAG's internal states: the newly added `SelectionDAG::getTopologicallyOrderedNodes` is a const member function that returns a list of all nodes in their topological order.
There are many cases where SDNodes shown in the
-debug-only=isel/isel-dump
of a DAG -- especially a big DAG -- just randomly spread everywhere, which can be really really painful to read sometimes.This patch adds a flag to sort DAGs before printing them in those debug prints. Note that sorted DAGs might have different schedulings and hence different results compared to the original one, but I thought it's still useful for debugging many of the (codegen) cases.