Richard Kenner via llvm-dev
2021-Feb-18 17:16 UTC
[llvm-dev] Potentially unsafe loop optimization
> 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.
Johannes Doerfert via llvm-dev
2021-Feb-18 17:25 UTC
[llvm-dev] Potentially unsafe loop optimization
I guess, Craig solved the mystery, nice :) On 2/18/21 11:16 AM, Richard Kenner wrote:>> 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.FWIW, It's not the position but the use of the addition that is the problem. As described in the beginning, computing poison is fine but if you use it to branch you are in trouble. That said, don't forget the initial UB in your example, I think you got the addresses wrong at some point and therefore write into the integers not the 3 byte array. ~ Johannes>> 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. >