[PATCH] D121415: [FunctionAttrs] Infer argmemonly .

[Nikita Popov via Phabricator via llvm-commits]

nikic 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)) {
----------------
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.


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


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


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121415