[PATCH] D26781: [LSR] Canonicalize formula and put recursive Reg related with current loop in ScaledReg.

Anna Thomas via llvm-commits llvm-commits at lists.llvm.org
Thu May 18 10:02:58 PDT 2017


Hi Wei,

Thanks for looking into this. Is this already fixed? 


Anna

> On May 17, 2017, at 5:48 PM, Wei Mi <wmi at google.com> wrote:
> 
> Quentin, I think you are right.
> 
> Here is what is happening:
> 
> This is the formula of a LSR Use, where multiple induction variables
> for the same loop appears in the same formula.
> 
> Initial formula of LSR Use. Look at the first Reg, the SCEVAddExprRec
> is inside an SCEVMulExpr, so it is not interesting as for IVUser
> analysis. The second SCEVAddExprRec is interesting for IVUser
> analysis. That is why the use is considered as a IVUse.
> LSR Use: Kind=Basic, Offsets={0}, widest fixup type: i32
> reg(((2 * ((-2 * %5) + (-1 * %8) + {(2 + (3 *
> undef)),+,8}<%not_zero48.us>)) + (-1 * %11))) + 1*reg({(3 +
> undef),+,8}<%not_zero48.us>)
> 
> In GenerateAllReuseFormulae, it becomes:
> reg(((-4 * %5) + (-2 * %8) + (-1 * %11))) + reg((3 + undef)) + reg((6
> * undef)) + reg({0,+,8}<%not_zero48.us>) +
> 1*reg({4,+,16}<%not_zero48.us>)
> Now there are two interesting SCEVAddExprRec from the same loop.
> 
> In GenerateTruncates, it tries to signextend the ScaledReg of the formula:
> reg((zext i32 ((-4 * %5) + (-2 * %8) + (-1 * %11)) to i64)) +
> reg((zext i32 (3 + undef) to i64)) + reg((zext i32 (6 * undef) to
> i64)) + reg({0,+,8}<nw><%not_zero48.us>) + 1*reg((4 + (sext i32
> {0,+,16}<%not_zero48.us> to i64))<nsw>)
> 
> After sign-extension, the ScaledReg in the formula is not a
> SCEVAddRecExpr anymore. However we have another SCEVAddRecExpr type of
> reg: reg({0,+,8}<nw><%not_zero48.us>), so canonicalization think maybe
> it is better to use reg({0,+,8}<nw><%not_zero48.us>) as the ScaledReg
> in the formula.
> 
> I think it makes sense to add a canonicalization call in
> GenerateTruncates for it to be more robust to handle special case like
> this. I am not sure how common the case is, as a possible enhancement
> for canonicalization, merging multiple SCEVAddExprRec from the same
> loop may be considered.
> 
> With the information above, if it is still ok, I am going to add the
> canonicalization call in GenerateTruncates, and a testcase.
> 
> Thanks,
> Wei.
> 
> On Wed, May 17, 2017 at 11:47 AM, Quentin Colombet <qcolombet at apple.com> wrote:
>> I am guessing we are missing a call to canonicalize after we changed a formula.
>> 
>>> On May 17, 2017, at 11:45 AM, Wei Mi <wmi at google.com> wrote:
>>> 
>>> Hi Anna,
>>> 
>>> Sorry to cause the regression. I am looking at the testcase right now
>>> you provided in the PR. I will revert the change if I cannot find a
>>> quick fix.
>>> 
>>> Wei.
>>> 
>>> On Wed, May 17, 2017 at 10:54 AM, Anna Thomas via Phabricator
>>> <reviews at reviews.llvm.org> wrote:
>>>> anna added a comment.
>>>> 
>>>> This patch caused an assertion failure as filed in https://bugs.llvm.org/show_bug.cgi?id=33077
>>>> @wmi: Could you please revert this change?
>>>> 
>>>> 
>>>> Repository:
>>>> rL LLVM
>>>> 
>>>> https://reviews.llvm.org/D26781
>>>> 
>>>> 
>>>> 
>> 



More information about the llvm-commits mailing list