[PATCH] D144468: [GlobalOpt] Extend CleanupPointerRootUsers to handle CE users.

[Florian Hahn via Phabricator via llvm-commits]

fhahn marked an inline comment as done.
fhahn added inline comments.


================
Comment at: llvm/lib/Transforms/IPO/GlobalOpt.cpp:247
+      } else
+        append_range(Worklist, CE->users());
     } else if (Constant *C = dyn_cast<Constant>(U)) {
----------------
nikic wrote:
> fhahn wrote:
> > nikic wrote:
> > > Do we know that the users here are only cast/GEP?
> > It could be any constant expression, but would that be a problem? I think the existing code should handle any type of constant expressions already. I added some extra tests with different constant expressions in  c34936465c56 and  7f51145b1bc2
> To give a silly example `store i32 0, ptr inttoptr (i64 add (i64 ptrtoint ptr @g to i64, i64 100)) to ptr)` could be writing to a different underlying object (without UB if certain conditions are met).
At the moment, earlier global analysis will fail if we encounter such constant expressions, but it is probably safest to just limit things to GEPOperator constant expressions here for now to avoid potentially breaking things in the future if the analysis may change. Updated the patch and also removed the `Visited` set which shouldn't be needed with the restricted version.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144468