[PATCH] D88891: [DebugInfo][InstrRef][1/4] Support transformations that widen or narrow defined values

[Stephen Tozer via Phabricator via llvm-commits]

StephenTozer added inline comments.


================
Comment at: llvm/include/llvm/CodeGen/MachineFunction.h:486
+  void makeDebugValueSubstitution(DebugInstrOperandPair, DebugInstrOperandPair,
+                                  unsigned SubReg = 0);
 
----------------
Also not so much a nit or request as a question; should this default value be taken to mean that it is okay for the substitution's subreg to be larger than the source operand, with such a case being treated as a no-op rather than a sext/zext? I'd assume so, since I don't believe that we would ever want to produce a sext/zext using this substitution mechanism - that should only happen as a result of salvaging.


================
Comment at: llvm/lib/Target/X86/X86FixupBWInsts.cpp:381
+  if (unsigned OldInstrNum = MI->peekDebugInstrNum()) {
+    unsigned Subreg = TRI->getSubRegIndex(MIB->getOperand(0).getReg(),
+                                          MI->getOperand(0).getReg());
----------------
Also one more question about the intended behaviour (that does not block this patch at all), it's potentially expected that we may have to recurse through several substitutions? As is I don't think there's any issue with that, since I believe there is a small lower bound on the number of substitutions we could ever need to make for a single reference, and it seems easier than checking for existing substitutions everywhere that we might create one.


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

https://reviews.llvm.org/D88891