Skip to content
This repository was archived by the owner on Feb 5, 2019. It is now read-only.

Commit 7513452

Browse files
Weiming Zhaoalexcrichton
Weiming Zhao
authored andcommitted
Fix PR25339: ARM Constant Island
Summary: Currently, the ARM Constant Island may not converge (or not converge quickly). This patch let it move to the closest water after the user if it doesn't converge after 15 iterations. This address https://llvm.org/bugs/show_bug.cgi?id=25339 Reviewers: t.p.northover, srhines, kristof.beyls, aadg, rengolin Subscribers: weimingz, aemerson, rengolin, llvm-commits Differential Revision: http://reviews.llvm.org/D16890 git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@261665 91177308-0d34-0410-b5e6-96231b3b80d8
1 parent 80fab33 commit 7513452

File tree

1 file changed

+39
-9
lines changed

1 file changed

+39
-9
lines changed

lib/Target/ARM/ARMConstantIslandPass.cpp

+39-9
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,11 @@ static cl::opt<bool>
5353
AdjustJumpTableBlocks("arm-adjust-jump-tables", cl::Hidden, cl::init(true),
5454
cl::desc("Adjust basic block layout to better use TB[BH]"));
5555

56+
static cl::opt<unsigned>
57+
CPMaxIteration("arm-constant-island-max-iteration", cl::Hidden, cl::init(30),
58+
cl::desc("The max number of iteration for converge"));
59+
60+
5661
/// UnknownPadding - Return the worst case padding that could result from
5762
/// unknown offset bits. This does not include alignment padding caused by
5863
/// known offset bits.
@@ -293,10 +298,10 @@ namespace {
293298
unsigned getCombinedIndex(const MachineInstr *CPEMI);
294299
int findInRangeCPEntry(CPUser& U, unsigned UserOffset);
295300
bool findAvailableWater(CPUser&U, unsigned UserOffset,
296-
water_iterator &WaterIter);
301+
water_iterator &WaterIter, bool CloserWater);
297302
void createNewWater(unsigned CPUserIndex, unsigned UserOffset,
298303
MachineBasicBlock *&NewMBB);
299-
bool handleConstantPoolUser(unsigned CPUserIndex);
304+
bool handleConstantPoolUser(unsigned CPUserIndex, bool CloserWater);
300305
void removeDeadCPEMI(MachineInstr *CPEMI);
301306
bool removeUnusedCPEntries();
302307
bool isCPEntryInRange(MachineInstr *MI, unsigned UserOffset,
@@ -456,8 +461,11 @@ bool ARMConstantIslands::runOnMachineFunction(MachineFunction &mf) {
456461
DEBUG(dbgs() << "Beginning CP iteration #" << NoCPIters << '\n');
457462
bool CPChange = false;
458463
for (unsigned i = 0, e = CPUsers.size(); i != e; ++i)
459-
CPChange |= handleConstantPoolUser(i);
460-
if (CPChange && ++NoCPIters > 30)
464+
// For most inputs, it converges in no more than 5 iterations.
465+
// If it doens't end in 10, the input may have huge BB or many CPEs.
466+
// In this case, we will try differnt heuristics.
467+
CPChange |= handleConstantPoolUser(i, NoCPIters >= CPMaxIteration / 2);
468+
if (CPChange && ++NoCPIters > CPMaxIteration)
461469
report_fatal_error("Constant Island pass failed to converge!");
462470
DEBUG(dumpBBs());
463471

@@ -1285,11 +1293,27 @@ static inline unsigned getUnconditionalBrDisp(int Opc) {
12851293
/// move to a lower address, so search backward from the end of the list and
12861294
/// prefer the first water that is in range.
12871295
bool ARMConstantIslands::findAvailableWater(CPUser &U, unsigned UserOffset,
1288-
water_iterator &WaterIter) {
1296+
water_iterator &WaterIter,
1297+
bool CloserWater) {
12891298
if (WaterList.empty())
12901299
return false;
12911300

12921301
unsigned BestGrowth = ~0u;
1302+
// The nearest water without splitting the UserBB is right after it.
1303+
// If the distance is still large (we have a big BB), then we need to split it
1304+
// if we don't converge after certain iterations. This helps the following
1305+
// situation to converge:
1306+
// BB0:
1307+
// Big BB
1308+
// BB1:
1309+
// Constant Pool
1310+
// When a CP access is out of range, BB0 may be used as water. However,
1311+
// inserting islands between BB0 and BB1 makes other accesses out of range.
1312+
MachineBasicBlock *UserBB = U.MI->getParent();
1313+
unsigned MinNoSplitDisp =
1314+
BBInfo[UserBB->getNumber()].postOffset(getCPELogAlign(U.CPEMI));
1315+
if (CloserWater && MinNoSplitDisp > U.getMaxDisp() / 2)
1316+
return false;
12931317
for (water_iterator IP = std::prev(WaterList.end()), B = WaterList.begin();;
12941318
--IP) {
12951319
MachineBasicBlock* WaterBB = *IP;
@@ -1301,6 +1325,8 @@ bool ARMConstantIslands::findAvailableWater(CPUser &U, unsigned UserOffset,
13011325
// should be relatively uncommon and when it does happen, we want to be
13021326
// sure to take advantage of it for all the CPEs near that block, so that
13031327
// we don't insert more branches than necessary.
1328+
// When CloserWater is true, we try to find the lowest address after (or
1329+
// equal to) user MI's BB no matter of padding growth.
13041330
unsigned Growth;
13051331
if (isWaterInRange(UserOffset, WaterBB, U, Growth) &&
13061332
(WaterBB->getNumber() < U.HighWaterMark->getNumber() ||
@@ -1312,8 +1338,11 @@ bool ARMConstantIslands::findAvailableWater(CPUser &U, unsigned UserOffset,
13121338
DEBUG(dbgs() << "Found water after BB#" << WaterBB->getNumber()
13131339
<< " Growth=" << Growth << '\n');
13141340

1315-
// Keep looking unless it is perfect.
1316-
if (BestGrowth == 0)
1341+
if (CloserWater && WaterBB == U.MI->getParent())
1342+
return true;
1343+
// Keep looking unless it is perfect and we're not looking for the lowest
1344+
// possible address.
1345+
if (!CloserWater && BestGrowth == 0)
13171346
return true;
13181347
}
13191348
if (IP == B)
@@ -1471,7 +1500,8 @@ void ARMConstantIslands::createNewWater(unsigned CPUserIndex,
14711500
/// is out-of-range. If so, pick up the constant pool value and move it some
14721501
/// place in-range. Return true if we changed any addresses (thus must run
14731502
/// another pass of branch lengthening), false otherwise.
1474-
bool ARMConstantIslands::handleConstantPoolUser(unsigned CPUserIndex) {
1503+
bool ARMConstantIslands::handleConstantPoolUser(unsigned CPUserIndex,
1504+
bool CloserWater) {
14751505
CPUser &U = CPUsers[CPUserIndex];
14761506
MachineInstr *UserMI = U.MI;
14771507
MachineInstr *CPEMI = U.CPEMI;
@@ -1494,7 +1524,7 @@ bool ARMConstantIslands::handleConstantPoolUser(unsigned CPUserIndex) {
14941524
MachineBasicBlock *NewIsland = MF->CreateMachineBasicBlock();
14951525
MachineBasicBlock *NewMBB;
14961526
water_iterator IP;
1497-
if (findAvailableWater(U, UserOffset, IP)) {
1527+
if (findAvailableWater(U, UserOffset, IP, CloserWater)) {
14981528
DEBUG(dbgs() << "Found water in range\n");
14991529
MachineBasicBlock *WaterBB = *IP;
15001530

0 commit comments

Comments
 (0)