[PATCH] D104268: [ptr_provenance] Introduce optional ptr_provenance operand to load/store

[Johannes Doerfert via Phabricator via llvm-commits]

jdoerfert accepted this revision.
jdoerfert added a comment.
This revision is now accepted and ready to land.

LG overall. Comments seem to be fine with this as well. Some nits on the style and order, can be addressed before the merge.



================
Comment at: llvm/docs/LangRef.rst:10096
+      store [volatile] <ty> <value>, <ty>* <pointer>[, ptr_provenance <ty>* <channel>][, align <alignment>][, !nontemporal !<nontemp_node>][, !invariant.group !<empty_node>]        ; yields void
+      store atomic [volatile] <ty> <value>, <ty>* <pointer>[, ptr_provenance <ty>* <channel>] [syncscope("<target-scope>")] <ordering>, align <alignment> [, !invariant.group !<empty_node>] ; yields void
       !<nontemp_node> = !{ i32 1 }
----------------
ptr_provenance is after syncscope in the ASM printer, also for loads. I would put ptr_provenance in the end honestly.


================
Comment at: llvm/include/llvm/IR/Instructions.h:301
+    else
+      return NoneType::None;
+  }
----------------
No else after return


================
Comment at: llvm/include/llvm/IR/Instructions.h:461-462
+      return getPtrProvenanceOperand();
+    else
+      return NoneType::None;
+  }
----------------
Nit: No else after return.


================
Comment at: llvm/lib/IR/Instructions.cpp:1513
 
+void LoadInst::setPtrProvenanceOperand(llvm::LoadInst::Value *Provenance) {
+  assert(Provenance && "Needs a provenance");
----------------



================
Comment at: llvm/lib/IR/Instructions.cpp:1607
 
+void StoreInst::setPtrProvenanceOperand(llvm::StoreInst::Value *Provenance) {
+  assert(Provenance && "Needs a provenance");
----------------



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

https://reviews.llvm.org/D104268