[PATCH] D58981: CodeGenPrepare: preserve inbounds attribute when sinking GEPs

[Dave Green via Phabricator via llvm-commits]

dmgreen accepted this revision.
dmgreen added a comment.
This revision is now accepted and ready to land.

Looks good to me then.



================
Comment at: llvm/lib/CodeGen/CodeGenPrepare.cpp:1975
+    // Conservatively reject 'inbounds' mismatches.
+    if (InBounds != other.InBounds)
+      return MultipleFields;
----------------
t.p.northover wrote:
> dmgreen wrote:
> > If one of the two is inbounds, does that not make the other inbounds? In other words, why would a mismatch in the inbounds flag matter?
> A violation would mean that one of the instructions produces `undef` and the other produces a known result, so I believe we could combine them by stripping off the `inbounds` attribute on the merged instruction.
> 
> But personally I doubt it's worth the effort. As far as I know, front-ends don't actually mix the two (all C and C++ arithmetic is inbounds or via inttoptr/ptrtoint).
I thought (correct me if I'm wrong) that a violation would produce poison. And because we know the results are used in a load/store, that would be UB. So we can presume that doesn't happen, and having inbounds on one implies inbounds on the other, providing they are otherwise identical.

I did see one example of this making a difference, although it was rather complex, quite a small change (and more often went up, not down). You are probably correct about it not being worth the effort.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D58981