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

[Johannes Doerfert via Phabricator via llvm-commits]

jdoerfert added a comment.

This looks generally fine to me. Let me know what you think about the two comments below.



================
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,
----------------
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.



================
Comment at: llvm/test/Transforms/Attributor/undefined_behavior.ll:139
+  ret void
+}
----------------
Maybe it makes sense to run the update_test_checks script on this file.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71787