[PATCH] D133307: [Assignment Tracking][14/*] Account for assignment tracking in instcombine

[Jeremy Morse via Phabricator via llvm-commits]

jmorse requested changes to this revision.
jmorse added a comment.
This revision now requires changes to proceed.

All round good; I've marked one of the questions that I really do have a bit of concern with -- is that a scenario that can actually happen? AFAIUI:

- Store instructions can be duplicated / unrolled as needed by llvm,
- They'll initially have the same DIAssignID,
- The marked code RAUWs the DIAssignID users, which could affect multiple dbg.assigns duplicated with the stores.

The answer to this may well be "no that doesn't happen", analysis appreciated though. Even if it does happen, I imagine that such scenarios are immediately optimised into not existing.



================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp:274-278
+    auto *FillVal = ConstantInt::get(ITy, Fill);
+    StoreInst *S = Builder.CreateStore(FillVal, Dest, MI->isVolatile());
+    S->copyMetadata(*MI, LLVMContext::MD_DIAssignID);
+    for (auto *DAI : at::getAssignmentMarkers(S))
+      DAI->setValue(FillVal);
----------------
jmorse wrote:
> Is there a risk that, if multiple stores have the same assign ID (which I think is possible?) that other dbg.assign intrinsics down other code paths will have their Values updated?
> 
> This might not actually be an issue, because if there's a call to memset (this context) then those parts won't have been promoted to Values.
^


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

https://reviews.llvm.org/D133307