[PATCH] D73763: [DSE] Lift post-dominance restriction.

[Johannes Doerfert via Phabricator via llvm-commits]

jdoerfert added inline comments.


================
Comment at: llvm/lib/Transforms/Scalar/DeadStoreElimination.cpp:1485
+        cast<Function>(M->getOrInsertFunction("____foobar", FnTy).getCallee());
+    State.ExitUseFn->addFnAttr(Attribute::ReadOnly);
+
----------------
fhahn wrote:
> jdoerfert wrote:
> > I'm not sure this is allowed in a function pass.
> Do you mean adding a new global or adding a function attribute to the added global? 
> 
> Ideally function passes would only modify things in the  function scope, but I think adding a new global is quite common, as most function passes that add new calls also may need to add a declaration of the called function.
> 
> As for adding the attribute, this needs to definitely change before submitting! I think we can either use an intrinsic (with the right attributes) or model it directly in MemorySSA, whatever option is preferred. 
> 
> 
I thought there was a new function created, sorry.

I have the feeling this "exit read" should be part of MSSAs functionality (as it seems tied to MSSA and reusable) but I don't have a strong opinion about it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73763