[PATCH] D117223: [GlobalOpt] Make global SRA offset based

Nikita Popov via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Jan 14 08:48:24 PST 2022


nikic added a comment.

In D117223#3243963 <https://reviews.llvm.org/D117223#3243963>, @reames wrote:

> Hm, your new code looks correct to me.
>
> There's one case in the original code I don't see reflected in the new code though.  It doesn't appear to have any tests, and I'm not 100% sure it's reachable at all, but consider the following.
>
> An outer struct with an inner array.  The index expression has a non-constant operand for the offset into the array.  (So, gep g, 0, v)  I believe the existing code will try to strip the struct off, and replace the global with the array.  Your code, because of the restriction to entirely constant offsets, will not.

The old code doesn't handle this either, because isSafeSROAGEP() checks that all indices are constant integers. Besides, doing this optimization without constant indices would be illegal, because there may be notional overindexing (this would only be legal with `inrange` annotations, which is not something the old code tries to handle -- GlobalSplit is the optimization that does).


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

https://reviews.llvm.org/D117223



More information about the llvm-commits mailing list