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

Vedant Kumar via llvm-commits llvm-commits at lists.llvm.org
Mon Apr 23 11:12:56 PDT 2018


> On Apr 21, 2018, at 12:55 AM, Francis Visoiu Mistrih <francisvm at yahoo.com> wrote:
> 
> Hi Vedant,
> 
>> On 21 Apr 2018, at 01:32, Vedant Kumar <vsk at apple.com <mailto: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.

That looks perfect, thanks a lot!

vedant

> 
> 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 <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/20180423/3a71f6f4/attachment.html>


More information about the llvm-commits mailing list