[llvm] r322936 - [CodeGen] Unify printing format of debug-location in both MIR and -debug

Francis Visoiu Mistrih via llvm-commits llvm-commits at lists.llvm.org
Sat Apr 21 00:55:09 PDT 2018


Hi Vedant,

> On 21 Apr 2018, at 01:32, Vedant Kumar <vsk at apple.com> wrote:
> 
> Hi Francis,
> 
> I'm debugging an issue in selection-dag which results in strange stepping behavior for swift code (rdar://33755881 <rdar://33755881>).
> 
> Before this patch, the output of -print-after-all would show this:
> 
> 	MOV16mr %rbp, 1, %noreg, -112, %noreg, killed renamable %ax; mem:ST2[%92] dbg:stepping.swift:8:17
> 	renamable %edx = MOVZX32rm16 %rbp, 1, %noreg, -112, %noreg; mem:LD2[%93](dereferenceable) dbg:stepping.swift:10:17
> 
> It's clear to see where the bug is based on this output, because we'd step from line 8 to line 10 and skip line 9.
> 
> After this patch, it's harder to identify the issue:
> 
>   MOV16mr $rbp, 1, $noreg, -112, $noreg, killed renamable $ax, debug-location !56 :: (store 2 into %ir.._value12)
>   renamable $edx = MOVZX32rm16 $rbp, 1, $noreg, -112, $noreg, debug-location !62 :: (dereferenceable load 2 from %ir.._value13)
> 
> The problem is that it's not immediately clear that locations 56 and 62 are on different lines.

I agree, sorry about that.

> 
> Would you be open to bringing the old kind of output back (possibly under a flag)?

Sure. How about keeping both? Something like:

MOV16mr $rbp, 1, $noreg, -112, $noreg, killed renamable $ax, debug-location !56 :: (store 2 into %ir.._value12) ; stepping.swift:10:17
  renamable $edx = MOVZX32rm16 $rbp, 1, $noreg, -112, $noreg, debug-location !62 :: (dereferenceable load 2 from %ir.._value13) ; stepping.swift:10:17

The main goal here was to be able to copy-paste MI from -print-after-all to MIR files easily. Even though it’s not completely true today, it’s still pretty close.

I’ll put up a patch for that if it works for you.

Thanks,

— 
Francis

> 
> vedant
> 
>> On Jan 19, 2018, at 3:44 AM, Francis Visoiu Mistrih via llvm-commits <llvm-commits at lists.llvm.org <mailto:llvm-commits at lists.llvm.org>> wrote:
>> 
>> Author: thegameg
>> Date: Fri Jan 19 03:44:42 2018
>> New Revision: 322936
>> 
>> URL: http://llvm.org/viewvc/llvm-project?rev=322936&view=rev <http://llvm.org/viewvc/llvm-project?rev=322936&view=rev>
>> Log:
>> [CodeGen] Unify printing format of debug-location in both MIR and -debug
>> 
>> Use "debug-location" instead of "; dbg:" in MI::print.
>> 
>> Modified:
>>    llvm/trunk/lib/CodeGen/MIRPrinter.cpp
>>    llvm/trunk/lib/CodeGen/MachineInstr.cpp
>>    llvm/trunk/unittests/CodeGen/MachineInstrTest.cpp
>> 
>> Modified: llvm/trunk/lib/CodeGen/MIRPrinter.cpp
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/MIRPrinter.cpp?rev=322936&r1=322935&r2=322936&view=diff <http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/MIRPrinter.cpp?rev=322936&r1=322935&r2=322936&view=diff>
>> ==============================================================================
>> --- llvm/trunk/lib/CodeGen/MIRPrinter.cpp (original)
>> +++ llvm/trunk/lib/CodeGen/MIRPrinter.cpp Fri Jan 19 03:44:42 2018
>> @@ -686,11 +686,11 @@ void MIPrinter::print(const MachineInstr
>>     NeedComma = true;
>>   }
>> 
>> -  if (MI.getDebugLoc()) {
>> +  if (const DebugLoc &DL = MI.getDebugLoc()) {
>>     if (NeedComma)
>>       OS << ',';
>>     OS << " debug-location ";
>> -    MI.getDebugLoc()->printAsOperand(OS, MST);
>> +    DL->printAsOperand(OS, MST);
>>   }
>> 
>>   if (!MI.memoperands_empty()) {
>> 
>> Modified: llvm/trunk/lib/CodeGen/MachineInstr.cpp
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/MachineInstr.cpp?rev=322936&r1=322935&r2=322936&view=diff <http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/MachineInstr.cpp?rev=322936&r1=322935&r2=322936&view=diff>
>> ==============================================================================
>> --- llvm/trunk/lib/CodeGen/MachineInstr.cpp (original)
>> +++ llvm/trunk/lib/CodeGen/MachineInstr.cpp Fri Jan 19 03:44:42 2018
>> @@ -1425,6 +1425,15 @@ void MachineInstr::print(raw_ostream &OS
>>     }
>>   }
>> 
>> +  if (!SkipDebugLoc) {
>> +    if (const DebugLoc &DL = getDebugLoc()) {
>> +      if (!FirstOp)
>> +        OS << ',';
>> +      OS << " debug-location ";
>> +      DL->printAsOperand(OS, MST);
>> +    }
>> +  }
>> +
>>   bool HaveSemi = false;
>>   if (!memoperands_empty()) {
>>     if (!HaveSemi) {
>> @@ -1441,6 +1450,9 @@ void MachineInstr::print(raw_ostream &OS
>>     }
>>   }
>> 
>> +  if (SkipDebugLoc)
>> +    return;
>> +
>>   // Print debug location information.
>>   if (isDebugValue() && getOperand(e - 2).isMetadata()) {
>>     if (!HaveSemi)
>> @@ -1457,13 +1469,6 @@ void MachineInstr::print(raw_ostream &OS
>>     }
>>     if (isIndirectDebugValue())
>>       OS << " indirect";
>> -  } else if (SkipDebugLoc) {
>> -    return;
>> -  } else if (debugLoc && MF) {
>> -    if (!HaveSemi)
>> -      OS << ";";
>> -    OS << " dbg:";
>> -    debugLoc.print(OS);
>>   }
>> 
>>   OS << '\n';
>> 
>> Modified: llvm/trunk/unittests/CodeGen/MachineInstrTest.cpp
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/CodeGen/MachineInstrTest.cpp?rev=322936&r1=322935&r2=322936&view=diff <http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/CodeGen/MachineInstrTest.cpp?rev=322936&r1=322935&r2=322936&view=diff>
>> ==============================================================================
>> --- llvm/trunk/unittests/CodeGen/MachineInstrTest.cpp (original)
>> +++ llvm/trunk/unittests/CodeGen/MachineInstrTest.cpp Fri Jan 19 03:44:42 2018
>> @@ -14,6 +14,8 @@
>> #include "llvm/CodeGen/TargetInstrInfo.h"
>> #include "llvm/CodeGen/TargetLowering.h"
>> #include "llvm/CodeGen/TargetSubtargetInfo.h"
>> +#include "llvm/IR/DebugInfoMetadata.h"
>> +#include "llvm/IR/ModuleSlotTracker.h"
>> #include "llvm/Support/TargetRegistry.h"
>> #include "llvm/Support/TargetSelect.h"
>> #include "llvm/Target/TargetMachine.h"
>> @@ -244,4 +246,25 @@ TEST(MachineInstrExpressionTraitTest, Is
>> 
>>   checkHashAndIsEqualMatch(VD2PU, VD2PD);
>> }
>> +
>> +TEST(MachineInstrPrintingTest, DebugLocPrinting) {
>> +  auto MF = createMachineFunction();
>> +
>> +  MCOperandInfo OpInfo{0, 0, MCOI::OPERAND_REGISTER, 0};
>> +  MCInstrDesc MCID = {0, 1,       1,       0,       0, 0,
>> +                      0, nullptr, nullptr, &OpInfo, 0, nullptr};
>> +
>> +  LLVMContext Ctx;
>> +  DILocation *DIL = DILocation::get(Ctx, 1, 5, (Metadata *)nullptr, nullptr);
>> +  DebugLoc DL(DIL);
>> +  MachineInstr *MI = MF->CreateMachineInstr(MCID, DL);
>> +  MI->addOperand(*MF, MachineOperand::CreateReg(0, /*isDef*/ true));
>> +
>> +  std::string str;
>> +  raw_string_ostream OS(str);
>> +  MI->print(OS);
>> +  ASSERT_TRUE(
>> +      StringRef(OS.str()).startswith("%noreg = UNKNOWN debug-location "));
>> +}
>> +
>> } // end namespace
>> 
>> 
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at lists.llvm.org <mailto:llvm-commits at lists.llvm.org>
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
> 

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180421/22ff36ef/attachment.html>


More information about the llvm-commits mailing list