[PATCH][LoopStrengthReduce] Don't trim a formula that uses a subset of required registers

Andrew Trick atrick at apple.com
Fri Apr 25 13:39:11 PDT 2014


On Apr 18, 2014, at 1:13 PM, Adam Nemet <anemet at apple.com> wrote:

> Ping
> 
> On Apr 10, 2014, at 9:12 PM, Adam Nemet <anemet at apple.com> wrote:
> 
>> Consider this use from the new testcase:
>> 
>> LSR Use: Kind=ICmpZero, Offsets={0}, widest fixup type: i32
>>   reg({1000,+,-1}<nw><%for.body>)
>>   -3003 + reg({3,+,3}<nw><%for.body>)
>>   -1001 + reg({1,+,1}<nuw><nsw><%for.body>)
>>   -1000 + reg({0,+,1}<nw><%for.body>)
>>   -3000 + reg({0,+,3}<nuw><%for.body>)
>>   reg({-1000,+,1}<nw><%for.body>)
>>   reg({-3000,+,3}<nsw><%for.body>)
>> 
>> This is the last use we consider for a solution in SolveRecurse, so CurRegs is
>> a large set.  (CurRegs is the set of registers that are needed by the
>> previously visited uses in the in-progress solution.)
>> 
>> ReqRegs is {
>> {3,+,3}<nw><%for.body>,
>> {1,+,1}<nuw><nsw><%for.body>
>> }
>> 
>> This is the intersection of the regs used by any of the formulas for the
>> current use and CurRegs.
>> 
>> Now, the code requires a formula to contain *all* these regs (the comment is
>> simply wrong), otherwise the formula is immediately disqualified.  Obviously,
>> no formula for this use contains two regs so they will each get disqualified.
>> 
>> The fix modifies the check to allow the formula in this case.  The idea is
>> that neither of these formulae is introducing any new registers which is the
>> point of this early pruning as far as I understand.
>> 
>> In terms of set arithmetic, we now allow formulas whose used regs are a subset
>> of the required regs not just the other way around.
>> 
>> There are few more loops in the test-suite that are now successfully LSRed.  I
>> have benchmarked those and found very minimal change.
>> 
>> Fixes <rdar://problem/13965777>

Adam,

This fix looks great to me! Please go ahead and commit.

Thanks,
-Andy



More information about the llvm-commits mailing list