Re: [llvm-commits] CVS: llvm/lib/CodeGen/BranchFolding.cpp
BranchFolding.cpp updated: 1.65 - 1.66
---
Log message:
Throttle tail merging; handling blocks with large numbers of
predecessors
is too slow.
Thanks Dale,
Please make this a hidden command line option like loop-unswitch-
threshold.
-Chris
---
Diffs of the changes: (+6 -2)
BranchFolding.cpp |8 ++--
1 files changed, 6 insertions(+), 2 deletions(-)
Index: llvm/lib/CodeGen/BranchFolding.cpp
diff -u llvm/lib/CodeGen/BranchFolding.cpp:1.65 llvm/lib/CodeGen/
BranchFolding.cpp:1.66
--- llvm/lib/CodeGen/BranchFolding.cpp:1.65 Mon Jun 4 18:52:54 2007
+++ llvm/lib/CodeGen/BranchFolding.cppThu Jun 7 19:34:27 2007
@@ -564,6 +564,9 @@
return MadeChange;
}
+// Throttle for huge numbers of predecessors (compile speed problems)
+#define THRESHOLD 100
+
bool BranchFolder::TailMergeBlocks(MachineFunction MF) {
if (!EnableTailMerge) return false;
@@ -577,7 +580,8 @@
MergePotentials.push_back(std::make_pair(HashEndOfMBB(I,
2U), I));
}
// See if we can do any tail merging on those.
- MadeChange |= TryMergeBlocks(NULL, NULL);
+ if (MergePotentials.size() THRESHOLD)
+MadeChange |= TryMergeBlocks(NULL, NULL);
// Look at blocks (IBB) with multiple predecessors (PBB).
// We change each predecessor to a canonical form, by
@@ -599,7 +603,7 @@
// transformations.)
for (MachineFunction::iterator I = MF.begin(), E = MF.end(); I !
= E; ++I) {
-if (!I-succ_empty() I-pred_size() = 2) {
+if (!I-succ_empty() I-pred_size() = 2 I-pred_size()
THRESHOLD) {
MachineBasicBlock *IBB = I;
MachineBasicBlock *PredBB = prior(I);
MergePotentials.clear();
___
llvm-commits mailing list
llvm-commits@cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
___
llvm-commits mailing list
llvm-commits@cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
Re: [llvm-commits] CVS: llvm/lib/CodeGen/BranchFolding.cpp
Hi Dale, Arrange for only 1 of multiple branches to landing pad to be kept. you seem to assume that there is at most one landing pad block for a given MBB, i.e. that all invokes in the MBB unwind to the same landing pad. Are you sure that's true? Ciao, Duncan. ___ llvm-commits mailing list llvm-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
Re: [llvm-commits] CVS: llvm/lib/CodeGen/BranchFolding.cpp
Hi Dale, Arrange for only 1 of multiple branches to landing pad to be kept. it struck me that this patch seems to assume that if a successor is a landing pad then it is necessarily a landing pad for the current MBB. But couldn't it be the landing pad for some other MBB, and simply a normal successor for the current one? Ciao, Duncan. ___ llvm-commits mailing list llvm-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
Re: [llvm-commits] CVS: llvm/lib/CodeGen/BranchFolding.cpp
On Jun 1, 2007, at 12:56 PM, Duncan Sands wrote: Hi Dale, Arrange for only 1 of multiple branches to landing pad to be kept. That should be successors, not branches, but you've gotten past that it struck me that this patch seems to assume that if a successor is a landing pad then it is necessarily a landing pad for the current MBB. I wasn't intending to assume that; how so? But couldn't it be the landing pad for some other MBB, and simply a normal successor for the current one? It could; there is no way to tell from the CFG information. ___ llvm-commits mailing list llvm-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
Re: [llvm-commits] CVS: llvm/lib/CodeGen/BranchFolding.cpp
Hi Dale,
it struck me that this patch seems to assume that if a successor
is a landing pad then it is necessarily a landing pad for the
current MBB.
I wasn't intending to assume that; how so?
But couldn't it be the landing pad for some other
MBB, and simply a normal successor for the current one?
It could; there is no way to tell from the CFG information.
consider the code:
MachineBasicBlock::succ_iterator SI = MBB.succ_begin();
bool foundPad = false;
while (SI != MBB.succ_end()) {
if (*SI == DestA DestA == DestB) {
DestA = DestB = 0;
if ((*SI)-isLandingPad())
foundPad = true;
++SI;
} else if (*SI == DestA) {
DestA = 0;
if ((*SI)-isLandingPad())
foundPad = true;
++SI;
} else if (*SI == DestB) {
DestB = 0;
if ((*SI)-isLandingPad())
foundPad = true;
++SI;
} else if ((*SI)-isLandingPad() !foundPad) {
foundPad = true;
++SI;
} else {
// Otherwise, this is a superfluous edge, remove it.
MBB.removeSuccessor(SI);
MadeChange = true;
}
}
Here is the worrying bit:
} else if ((*SI)-isLandingPad() !foundPad) {
foundPad = true;
++SI;
} else {
Suppose a successor S1 has isLandingPad true because it is
a landing pad for some other MBB, and some later successor (S2)
has isLandingPad true because it is the landing pad for this
MBB. What will happen? When we get to S1 foundPad is set to
true and S1 is not deleted. Later we get to S2 and, because
foundPad is true, fall through to
} else {
// Otherwise, this is a superfluous edge, remove it.
MBB.removeSuccessor(SI);
MadeChange = true;
}
and wrongly delete S2.
Ciao,
Duncan.
___
llvm-commits mailing list
llvm-commits@cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
Re: [llvm-commits] CVS: llvm/lib/CodeGen/BranchFolding.cpp
On Jun 1, 2007, at 1:24 PM, Duncan Sands wrote:
Suppose a successor S1 has isLandingPad true because it is
a landing pad for some other MBB, and some later successor (S2)
has isLandingPad true because it is the landing pad for this
MBB. What will happen? When we get to S1 foundPad is set to
true and S1 is not deleted. Later we get to S2 and, because
foundPad is true, fall through to
} else {
// Otherwise, this is a superfluous edge, remove it.
MBB.removeSuccessor(SI);
MadeChange = true;
}
and wrongly delete S2.
Good catch, thanks. I'll fix that.
___
llvm-commits mailing list
llvm-commits@cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
Re: [llvm-commits] CVS: llvm/lib/CodeGen/BranchFolding.cpp
On Jun 1, 2007, at 1:28 PM, Dale Johannesen wrote:
On Jun 1, 2007, at 1:24 PM, Duncan Sands wrote:
Suppose a successor S1 has isLandingPad true because it is
a landing pad for some other MBB, and some later successor (S2)
has isLandingPad true because it is the landing pad for this
MBB. What will happen? When we get to S1 foundPad is set to
true and S1 is not deleted. Later we get to S2 and, because
foundPad is true, fall through to
} else {
// Otherwise, this is a superfluous edge, remove it.
MBB.removeSuccessor(SI);
MadeChange = true;
}
and wrongly delete S2.
Good catch, thanks. I'll fix that.
Should be fixed now.
___
llvm-commits mailing list
llvm-commits@cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
