[PATCH] D48676: [Local] replaceAllDbgUsesWith: Update debug values before RAUW

[Bjorn Pettersson via Phabricator via llvm-commits]

bjope added a comment.

FYI: Looks like you have address my earlier comments. So I have no blocking remarks.

The addition of DomPoint / DT was interesting (not sure that I have figured out exactly which scenarios that are handled)
I think that we still have the problem with only checking dominance of the moved/replaced value though.
As was discussed earlier somewhere (a patch in post RA MachineSink) we also might have the problem that there are other dbg.value intrinsics, referring to the same variable (or an overlapping fragment). And the question is if it is correct to reorder such dbg.value intrinsics, or what to do. Matt wrote a TR about it here https://bugs.llvm.org/show_bug.cgi?id=37897



================
Comment at: lib/Transforms/Utils/Local.cpp:1844
+    // access the low `FromBits` bits when inspecting the source variable.
+    if (FromBits < ToBits)
+      return rewriteDebugUsers(From, To, DomPoint, DT, Identity);
----------------
vsk wrote:
> gramanas wrote:
> > Could FromBits be equal to ToBits? In that case neither zext nor sext is needed. Could this be `if (FromBits <= ToBits)`?
> No, that case should be handled earlier, in isBitCastSemanticsPreserving().
It would still be more correct to enter the if in case FromBits==ToBits. So I don't think it hurts to use `<=` here.


https://reviews.llvm.org/D48676