[PATCH] D99424: [BasicAA] Be more careful with modulo ops on VariableGEPIndex.

[Mikael Holmén via Phabricator via llvm-commits]

uabelho added a comment.

In D99424#2851015 <https://reviews.llvm.org/D99424#2851015>, @fhahn wrote:

> Ok, after another look, I think the problem was that setting `Scale` as in this patch may turn `Scale` from a negative to a positive number, which in turn means that `AllNonNegative` may be set incorrectly, which should be independent of the scale used for the `GCD`.
>
> I pushed a test for that scenario (returned noalias based on all indices being non-negative) in f4ea6531e677 <https://reviews.llvm.org/rGf4ea6531e677b1a3c107d7009a7e2f195c8fa915>. And a fix to avoid nuking the sign of the original `Scale`: e6d22d0174e0 <https://reviews.llvm.org/rGe6d22d0174e09fa01342d9ed1dca47bc1eb58303> . This is likely only temporary, as relying on the sign of the scale is also problematic when the operations wrap.
>
> @nikic it would be great if you could take a quick look at the commits. I'll probably post a follow-up in the next few days for review.
>
> @mstorsjo @uabelho  could you check if e6d22d0174e0 <https://reviews.llvm.org/rGe6d22d0174e09fa01342d9ed1dca47bc1eb58303> fixes the issues you are seeing? I checked with @mstorsjo's example and with the new patch we get the same results as with D99424 <https://reviews.llvm.org/D99424> reverted.

It helps in my case. The miscompile goes away with e6d22d0174e0 <https://reviews.llvm.org/rGe6d22d0174e09fa01342d9ed1dca47bc1eb58303>. Thanks!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D99424/new/

https://reviews.llvm.org/D99424