[PATCH] D71787: [Attributor] UB Attribute now handles all instructions that access memory through a pointer

[Stefanos Baziotis via Phabricator via llvm-commits]

baziotis marked 2 inline comments as done.
baziotis added inline comments.


================
Comment at: llvm/lib/Transforms/IPO/Attributor.cpp:2033
 
-    A.checkForAllInstructions(InspectLoadForUB, *this, {Instruction::Load});
-    if (PrevSize != NoUBLoads.size())
+    A.checkForAllInstructions(InspectMemAccessInstForUB, *this,
+                              {Instruction::Load, Instruction::Store,
----------------
jdoerfert wrote:
> sstefan1 wrote:
> > I think you should be able to use `CheckForAllReadWriteInstructions` here. Then you don't need to add 2 additional cases in `initializeInformationCache`.
> We can do that, but you'll see other instructions here too. I'm fine with doing it step by step as well.
> 
> I think you should be able to use CheckForAllReadWriteInstructions here. Then you don't need to add 2 additional cases in initializeInformationCache.

Thanks Stefan! I hadn't noticed.

> We can do that, but you'll see other instructions here too. I'm fine with doing it step by step as well.

Alright, I'll leave it as it is and re-introduce a TODO to not only check these instructions (i.e. make clear that other instructions will be added).


================
Comment at: llvm/test/Transforms/Attributor/undefined_behavior.ll:139
+  ret void
+}
----------------
jdoerfert wrote:
> Maybe it makes sense to run the update_test_checks script on this file.
Thanks, I didn't know about it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71787