[PATCH] D76894: [GlobalOpt/GlobalStatus] Handle GlobalVariables passed as function call operands with access attributes

[Johannes Doerfert via Phabricator via llvm-commits]

jdoerfert requested changes to this revision.
jdoerfert added a comment.
This revision now requires changes to proceed.

Thanks for working on this, I'm really hoping we start to use attributes more aggressively. I inlined comments.



================
Comment at: llvm/lib/Transforms/IPO/GlobalOpt.cpp:1829
+        // call site, or ignore them if they are not going to be dereferenced.
+        assert(CB->isArgOperand(&U));
+        unsigned ArgNo = CB->getArgOperandNo(&U);
----------------
This assert would need a message but it has to go completely. Having non-arg-operand uses is totally fine. That said, we should have a test case. One example where this will soon happen in practise is `llvm.assume`. Create a test with something like:
`call void @llvm.assume(i1 true) ["align"(@Global, i64 128)]`
If the use is not an arg operand you have to bail. Except if the User `isDroppable()`.


================
Comment at: llvm/lib/Transforms/IPO/GlobalOpt.cpp:1877
           return DT.dominates(S, L) &&
                  DL.getTypeStoreSize(LTy) <= DL.getTypeStoreSize(STy);
         }))
----------------
This doesn't work for calls unfortunately. We don't have an attribute that says it is accessed and how much of it is (yet!). My suggestion is a TODO and the following handling for now:
Write-only uses in calls are ignored, as read-none uses are. Read-only uses are matched with the type of the global value, thus we pretend we might read the entire thing.


================
Comment at: llvm/lib/Transforms/Utils/GlobalStatus.cpp:185
+        } else
+          GS.IsLoaded = true;
       } else {
----------------
This handles operand bundle uses wrong. We need to bail for them (I guess), except if the user `isDroppable` (maybe).

---

If the argument is not marked `nocapture` you have to assume everything could happen as you cannot track uses anymore.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76894