[PATCH] D76759: [AssumeBundles] Preserve Information from Load/Store

[Johannes Doerfert via Phabricator via llvm-commits]

jdoerfert added a comment.

I love the progress :) Some comments below.



================
Comment at: llvm/include/llvm/IR/KnowledgeRetention.h:35
-}
+CallInst *BuildAssumeFromInst(Instruction *I);
 
 /// It is possible to have multiple Value for the argument of an attribute in
----------------
Feel free to commit this as NFC right away.


================
Comment at: llvm/lib/IR/KnowledgeRetention.cpp:210
+                MA.valueOrOne().value());
+    };
+    if (auto *Load = dyn_cast<LoadInst>(I))
----------------
Let's make this a member function like `addCall`. We should also merge this with existing logic and put it as a helper somewhere. Given an instruction, and potentially a set of attributes as filter, determine all attributes we can derive from the instruction. I might look into this as the attributor should use that as well to cut down on duplication.


================
Comment at: llvm/lib/IR/KnowledgeRetention.cpp:214
+    if (auto *Store = dyn_cast<StoreInst>(I))
+      AddMemInst(Store, Store->getValueOperand()->getType());
     // TODO: Add support for the other Instructions.
----------------
I'd switch on the opcodes, use `if-else-if`, or early returns.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76759