llvm dev - Nov 2013 - [LLVMdev] SCEV getMulExpr() not propagating Wrap flags

On 16 November 2013 06:56, Andrew Trick <atrick at apple.com> wrote:
> - getMulExpr constructs a new AddRec with NSW:
>
>       Flags = AddRec->getNoWrapFlags(clearFlags(Flags, SCEV::FlagNW));
>       const SCEV *NewRec = getAddRecExpr(NewOps, AddRecLoop, Flags);
>
Hi Andrew,

Thanks for looking at this.

Clearing the flags here makes sense, but it's being too conservative.
I'm
thinking I'll need something like propagateSafeFlags(...) with knowledge of
the loop induction ranges or something else instead of simply relying on
later processes to spot the problems.


The clearing of NW does not need to happen in your case (NSW/NUW
should
> always imply NW). You could try this and see if it fixes the problem:
>
>       Flags = AddRec->getNoWrapFlags();
>
> If that fixes your case, then the right/safe fix will be trivial.
>
It does, on that case. Shouldn't be too hard, but having the right info on
the right place could be tricky.

cheers,
--renato
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20131116/76c06e78/attachment.html>

> On Nov 16, 2013, at 4:11 AM, Renato Golin <renato.golin at
linaro.org> wrote:
> 
>> On 16 November 2013 06:56, Andrew Trick <atrick at apple.com>
wrote:
>> - getMulExpr constructs a new AddRec with NSW:
>> 
>>       Flags = AddRec->getNoWrapFlags(clearFlags(Flags,
SCEV::FlagNW));
>>       const SCEV *NewRec = getAddRecExpr(NewOps, AddRecLoop, Flags);
> 
> Hi Andrew,
> 
> Thanks for looking at this.
> 
> Clearing the flags here makes sense, but it's being too conservative.
I'm thinking I'll need something like propagateSafeFlags(...) with
knowledge of the loop induction ranges or something else instead of simply
relying on later processes to spot the problems.
> 
> 
>> The clearing of NW does not need to happen in your case (NSW/NUW should
always imply NW). You could try this and see if it fixes the problem:
>> 
>>       Flags = AddRec->getNoWrapFlags();
>> 
>> If that fixes your case, then the right/safe fix will be trivial.
> 
> It does, on that case. Shouldn't be too hard, but having the right info
on the right place could be tricky.
> 
> cheers,
> --
That code is not intending to drop information in your case, but the
implementation of the flags is sloppy here. If you can drop a test case in a PR
I'll fix it.

Andy
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20131116/6f355b13/attachment.html>

On 16 November 2013 19:59, Andrew Trick <atrick at apple.com>
wrote:
>
> That code is not intending to drop information in your case, but the
> implementation of the flags is sloppy here. If you can drop a test case in
> a PR I'll fix it.
>
Thanks Andrew,

http://llvm.org/bugs/show_bug.cgi?id=17963

cheers,
--renato
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20131116/738d50fb/attachment.html>