[PATCH] D106589: [GlobalOpt] support ConstantExpr use of global address for OptimizeGlobalAddressOfMalloc

[Shimin Cui via Phabricator via llvm-commits]

scui added inline comments.


================
Comment at: llvm/lib/Transforms/IPO/GlobalOpt.cpp:735
+        // Ignore stores to the global.
+        if (SI->getPointerOperand() != P)
+          return false;
----------------
efriedma wrote:
> Instead of `SI->getPointerOperand() != P`, should be `SI->getValueOperand() == P`?
SI->getPointerOperand() != P is alright. It's ok to store into global (actually the StoredOnce).


================
Comment at: llvm/lib/Transforms/Utils/GlobalStatus.cpp:111
+            Ptr = Ptr->stripPointerCasts();
+          if (const GlobalVariable *GV = dyn_cast<GlobalVariable>(Ptr)) {
             Value *StoredVal = SI->getOperand(0);
----------------
efriedma wrote:
> Is this going to affect other transforms that use the result of analyzeGlobalAux?
I only see one user of GlobalStatus - that is processGlobal in GlobalOpt.cpp. I checked all the transformations under this function. Other than tryToOptimizeStoreOfMallocToGlobal,  this added code should be ok for all other transformations, either ConstantExpr is handled already or bailout early if there is any use of ConstantExpr. There is only one to bailout early - TryToShrinkGlobalToBoolean -  I'll leave this as it is for now as I don't see any real case for this with ConstantExpr. 



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

https://reviews.llvm.org/D106589