[PATCH] D82678: [CGP] Set debug locations when optimizing phi types

[Dave Green via Phabricator via llvm-commits]

dmgreen added inline comments.


================
Comment at: llvm/lib/CodeGen/CodeGenPrepare.cpp:5925-5933
+    SmallVector<DbgValueInst *, 1> DbgValues;
+    findDbgValues(DbgValues, Phi);
+    for (DbgValueInst *DVI : DbgValues) {
+      // Bitcasts are apparently irrelevant for debug info. Rewrite the dbg.value
+      // to use the new phi.
+      DVI->setOperand(0, MetadataAsValue::get(Phi->getContext(),
+                                              ValueAsMetadata::get(NewPhi)));
----------------
jmorse wrote:
> I'm not overly familiar with this optimisation, but won't the debug users of Phi be RAUW'd to the bitcast by the next loop? If so, there Shouldn't (TM) be a need to explicitly point them at the new PHI.
The offending part is down in CodeGenPrepare::optimizePhiTypes where we call I->replaceAllUsesWith(UndefValue::get(I->getType())).  Because we are dealing with a collection of phi nodes, that may form a loop, we need to ensure they really have no users before we erase them.

And we are converting the type of the phis from float to int, so don't RAUW on them directly. We add new phi nodes and remove the old.

I'm certainly not a debug expert and if this code is going to cause more trouble that it's worth we can remove it. But I think (unconfidently) that it's needed to keep the debug values present and connected.


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

https://reviews.llvm.org/D82678