[PATCH] D121415: [FunctionAttrs] Infer argmemonly .

[Florian Hahn via Phabricator via llvm-commits]

fhahn marked 3 inline comments as done.
fhahn added inline comments.


================
Comment at: llvm/lib/Transforms/IPO/FunctionAttrs.cpp:153
+    // analyzing calls, loads and stores, or if it needs updating later.
+    bool AccessesNonArgsOrAllocaUpdated = false;
     if (auto *Call = dyn_cast<CallBase>(&I)) {
----------------
nikic wrote:
> The way the code is structured, can't you put the fallback into an `else` at the end? It looks like all other branches update it.
Done, thanks!


================
Comment at: llvm/lib/Transforms/IPO/FunctionAttrs.cpp:265
+                                        : FMRB_OnlyReadsArgumentPointees)
+             : FMRB_DoesNotAccessMemory;
 }
----------------
nikic wrote:
> To avoid enumerating all combinations, may it would be cleaner to construct ModRefInfo and then or it with either FMRL_ArgumentPointees or FMRL_Anywhere?
Yes that's better, updated!


================
Comment at: llvm/lib/Transforms/IPO/FunctionAttrs.cpp:297
+      WritesMemory = true;
+    }
     if (FMRB == FMRB_DoesNotAccessMemory)
----------------
nikic wrote:
> I think you'd want to `ReadsMemory |= isRefSet(MR)` etc below and then early exit on `ReadMemory && WritesMemory && !ArgMemOnly` to avoid some duplication here.
Thanks, using isRefSet/isModSet works well!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121415