[PATCH] D132223: [Assignment Tracking][4/*] Add llvm.dbg.assign intrinsic boilerplate

Orlando Cazalet-Hyams via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Sep 6 06:10:22 PDT 2022


Orlando added a comment.

Thanks for all the reviews. I just wanted to check something (inline) before updating this patch.



================
Comment at: llvm/lib/IR/IntrinsicInst.cpp:86
+      return false;
+    DAI->setAddress(NewValue);
+    return true;
----------------
jmorse wrote:
> Hmmmm. So this is called _inside_ an assertion, but it also has a side-effect / assignment... doesn't that mean we'll get different behaviour if LLVM is compiled with/without assertions?
`DbgAssignAddrReplaced ` is a bool assigned the result of the in-line call to the lambda. I can re-arrange this to get rid of the lambda if you'd prefer?


================
Comment at: llvm/lib/IR/IntrinsicInst.cpp:179-184
+void DbgAssignIntrinsic::replaceUsesOfWith(Value *From, Value *To) {
+  if (getValue() == From)
+    setValue(To);
+  if (getAddress() == From)
+    setAddress(To);
+}
----------------
jmorse wrote:
> Will this work with DIArgList's? (Probably, figured I'd ask though). Seeing how DbgAddrInst is acting as a DbgValueInst, AFAIUI the loop-strength-reduce and similar code will try and RAUW the value with a DIArgList of values occasionally. I think the semantics of this is obvious, the only risk is that we encode assumptions that the Value is only ever a single ValueAsMetadata.
Ah, sorry, I had intended to remove `replaceUsesOfWith` and replace calls with `replaceVariableLocationOp`. I must've missed that change from this patch.


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

https://reviews.llvm.org/D132223



More information about the llvm-commits mailing list