llvm dev - Feb 2021 - Potentially unsafe loop optimization

On Thu, Feb 18, 2021 at 5:16 AM Richard Kenner <kenner at adacore.com>
wrote:
> > Looks like the expand memcmp pass expanded the bcmp. Then ran
> > InstructionSimplify on every basic block in the function because a
> > change was made. That decided that the compare was always false. But
> > I'm not sure it had anything to do with the bcmp expansion.
>
> But that's not the relevant compare.  That compare is actually
> provably false (abort is never called).  The compare that's at issue
> is the loop end compare.
>
Sorry, I wasn't clear on which compare. I was referring to the loop end
compare. After the loop strength reduce pass in the llc codegen pipeline we
have this IR.

%c.0 = phi i8 [ 0, %entry ], [ %next.loop.var, %loop.cond.iter ]
...
%next.loop.var = add nuw i8 %c.0, 1
%loop.iter.cond = icmp eq i8 %next.loop.var, 0

Then the memcmp expand pass runs and expands the bcmp. Because this made a
change to the IR, it runs InstructionSimplify on every basic block in the
function. Including basic blocks that didn't contain the bcmp.
InstructionSimplify notices that loop end icmp uses an add nuw and phi that
started at 0. Since the nuw says it can never get back to zero, the icmp is
replaced with false.

Had the bcmp not been there, this late run of InstructionSimplify wouldn't
have happened. There are no other optimizations that always run in this
part of the pipeline that would see the bad IR created from loop strength
reduce and optimize based on it. SelectionDAG can do optimizations with
nuw, but it only runs on a single basic block so it won't see the phi.

Nikita's patch, 835104a1141a06ae7821fe2b642b9603e00aa17b removes the nuw
from the add on trunk. I didn't look at why llvm 10 works.
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20210218/67c16978/attachment.html>

> Had the bcmp not been there, this late run of InstructionSimplify
> wouldn't have happened. There are no other optimizations that always
> run in this part of the pipeline that would see the bad IR created
> from loop strength reduce and optimize based on it. SelectionDAG can
> do optimizations with nuw, but it only runs on a single basic block
> so it won't see the phi.
OK, I understand now.
> Nikita's patch, 835104a1141a06ae7821fe2b642b9603e00aa17b removes the
> nuw from the add on trunk.
That sounds right given that it moved the add before the branch.
> I didn't look at why llvm 10 works.
Probably because it didn't do the late run or it didn't spot the phi
would be my guess.

> Nikita's patch, 835104a1141a06ae7821fe2b642b9603e00aa17b removes the
> nuw from the add on trunk. I didn't look at why llvm 10 works.
Note that when I apply just that patch to LLVM 11.0.1, it doesn't seem
to do anything with this test.  Is that expected?