[PATCH] D101245: [GlobalOpt] Disable heap SROA when GEP of the only storer is used

[Arthur Eubanks via Phabricator via llvm-commits]

aeubanks added inline comments.


================
Comment at: llvm/lib/Transforms/IPO/GlobalOpt.cpp:1016
-    // Must index into the array and into the struct.
-    if (isa<GetElementPtrInst>(Inst) && Inst->getNumOperands() >= 3) {
-      if (!ValueIsOnlyUsedLocallyOrStoredToOneGlobal(Inst, GV, PHIs))
----------------
MaskRay wrote:
> aeubanks wrote:
> > seems like this doesn't take into account when the global variable is an array.
> > normally, if we just have a normal struct, the GEP's first operand is the GV, second operand is something like 0, and the third operand indexes into the struct itself, which is fine for SROA. having fewer than 3 operands can directly reference the struct
> > with an array, we need to account for an extra index to index into the array
> > 
> Do you have specific suggestion to the diff? :)
> 
> I am thinking the value of array heap SROA is low but I don't want to just remove the implementation, so drop GEP - see the tests like heap-sra-phi.ll, heap SROA still works if the function allocates and forgets the array.
maybe check the GV type to see if it's an array, and in that case use 4 instead of 3?
supporting GEPs seems natural, I'm not sure why we wouldn't want to, seems like code would probably use GEPs into global values to do things


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101245