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

[Johannes Doerfert via Phabricator via llvm-commits]

jdoerfert added a comment.

I only have coding style comments left. Providing the logic in a more general way will be future work.



================
Comment at: llvm/lib/IR/KnowledgeRetention.cpp:189
+    AssumedKnowledgeSet.insert(AK);
+  };
+
----------------
Nit:
We can have non pointer attributes (later).
-`Ptr`
+`V` or `Val`
(Then we'll also need to rename `setPointer`)


================
Comment at: llvm/lib/IR/KnowledgeRetention.cpp:200
+                                  MemInst->getPointerOperand()
+                                      ->getType()
+                                      ->getPointerAddressSpace()))
----------------
The above two lines is `T`, isn't it?


================
Comment at: llvm/lib/IR/KnowledgeRetention.cpp:213
+      Impl(Store, Store->getValueOperand()->getType());
+  }
+
----------------
Style: TBH, I'm not necessarily a fan of the overloading via lambda idea. Couldn't we pass all necessary information to `addLoadOrStore`. I mean, we make it `addAccessedPtr(Instruction &Inst, Value &Ptr, Type &AccTy, MaybeAlign Alignment)`
and pass the appropriate values.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76759