[llvm] r227481 - DebugInfo: Teach Fast ISel to respect the debug location of comparisons in jumps
David Blaikie
dblaikie at gmail.com
Thu Jan 29 11:50:19 PST 2015
On Thu, Jan 29, 2015 at 11:20 AM, Eric Christopher <echristo at gmail.com>
wrote:
> Agreed.
>
> Also, testcase?
>
Ah, thanks for the catch - forgot to git add. Committed it r227485.
> And the other fast-isel ports? :)
>
No reason to believe the other ports have the same bug, though I haven't
looked (I always get confused by the triple/target syntax, to target
another architecture (circular problem - don't do it often enough to be
familiar and avoid doing it due to unfamiliarity)). A quick search of
similar functions (such as 'FastEmitCompare') doesn't turn up any results
in the other FastISel implementations... I see, different naming
conventions, emitCmp, SelectCmp, EmitCmp...
But yeah, generally suffer from the same bug. (though I'm not sure how much
value there is in fixing up this one case given the breadth of the issue
here - removing DbgLoc and changing all uses to explicitly handle DebugLocs
from Instruction to MachineInstr is going to be necessary to make this
stuff better - though I don't plan on doing that personally any time soon)
I'll poke at the other targets, see if there's a reasonable test case
generalization (maybe if I'm lucky they'll all have 'cmp' somewhere in the
instruction name? ;)).
>
> -eric
>
> On Thu Jan 29 2015 at 11:16:03 AM David Blaikie <dblaikie at gmail.com>
> wrote:
>
>> Author: dblaikie
>> Date: Thu Jan 29 13:09:18 2015
>> New Revision: 227481
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=227481&view=rev
>> Log:
>> DebugInfo: Teach Fast ISel to respect the debug location of comparisons
>> in jumps
>>
>> The use of the DbgLoc in FastISel is probably something we should fix.
>> It's prone to leaking the wrong location into instructions - we should
>> have a clear chain of custody from the debug location of an IR
>> Instruction to that of a MachineInstr to avoid such leakage.
>>
>> Modified:
>> llvm/trunk/lib/Target/X86/X86FastISel.cpp
>>
>> Modified: llvm/trunk/lib/Target/X86/X86FastISel.cpp
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/
>> X86/X86FastISel.cpp?rev=227481&r1=227480&r2=227481&view=diff
>> ============================================================
>> ==================
>> --- llvm/trunk/lib/Target/X86/X86FastISel.cpp (original)
>> +++ llvm/trunk/lib/Target/X86/X86FastISel.cpp Thu Jan 29 13:09:18 2015
>> @@ -80,7 +80,7 @@ public:
>> #include "X86GenFastISel.inc"
>>
>> private:
>> - bool X86FastEmitCompare(const Value *LHS, const Value *RHS, EVT VT);
>> + bool X86FastEmitCompare(const Value *LHS, const Value *RHS, EVT VT,
>> DebugLoc DL);
>>
>> bool X86FastEmitLoad(EVT VT, const X86AddressMode &AM,
>> MachineMemOperand *MMO,
>> unsigned &ResultReg);
>> @@ -1109,7 +1109,7 @@ static unsigned X86ChooseCmpImmediateOpc
>> }
>>
>> bool X86FastISel::X86FastEmitCompare(const Value *Op0, const Value *Op1,
>> - EVT VT) {
>> + EVT VT, DebugLoc CurDbgLoc) {
>> unsigned Op0Reg = getRegForValue(Op0);
>> if (Op0Reg == 0) return false;
>>
>> @@ -1122,7 +1122,7 @@ bool X86FastISel::X86FastEmitCompare(con
>> // CMPri, otherwise use CMPrr.
>> if (const ConstantInt *Op1C = dyn_cast<ConstantInt>(Op1)) {
>> if (unsigned CompareImmOpc = X86ChooseCmpImmediateOpcode(VT, Op1C))
>> {
>> - BuildMI(*FuncInfo.MBB, FuncInfo.InsertPt, DbgLoc,
>> TII.get(CompareImmOpc))
>> + BuildMI(*FuncInfo.MBB, FuncInfo.InsertPt, CurDbgLoc,
>> TII.get(CompareImmOpc))
>> .addReg(Op0Reg)
>> .addImm(Op1C->getSExtValue());
>> return true;
>> @@ -1134,7 +1134,7 @@ bool X86FastISel::X86FastEmitCompare(con
>>
>> unsigned Op1Reg = getRegForValue(Op1);
>> if (Op1Reg == 0) return false;
>> - BuildMI(*FuncInfo.MBB, FuncInfo.InsertPt, DbgLoc, TII.get(CompareOpc))
>> + BuildMI(*FuncInfo.MBB, FuncInfo.InsertPt, CurDbgLoc,
>> TII.get(CompareOpc))
>> .addReg(Op0Reg)
>> .addReg(Op1Reg);
>>
>> @@ -1202,7 +1202,7 @@ bool X86FastISel::X86SelectCmp(const Ins
>>
>> ResultReg = createResultReg(&X86::GR8RegClass);
>> if (SETFOpc) {
>> - if (!X86FastEmitCompare(LHS, RHS, VT))
>> + if (!X86FastEmitCompare(LHS, RHS, VT, I->getDebugLoc()))
>> return false;
>>
>> unsigned FlagReg1 = createResultReg(&X86::GR8RegClass);
>> @@ -1227,7 +1227,7 @@ bool X86FastISel::X86SelectCmp(const Ins
>> std::swap(LHS, RHS);
>>
>> // Emit a compare of LHS/RHS.
>> - if (!X86FastEmitCompare(LHS, RHS, VT))
>> + if (!X86FastEmitCompare(LHS, RHS, VT, I->getDebugLoc()))
>> return false;
>>
>> BuildMI(*FuncInfo.MBB, FuncInfo.InsertPt, DbgLoc, TII.get(Opc),
>> ResultReg);
>> @@ -1353,7 +1353,7 @@ bool X86FastISel::X86SelectBranch(const
>> std::swap(CmpLHS, CmpRHS);
>>
>> // Emit a compare of the LHS and RHS, setting the flags.
>> - if (!X86FastEmitCompare(CmpLHS, CmpRHS, VT))
>> + if (!X86FastEmitCompare(CmpLHS, CmpRHS, VT, CI->getDebugLoc()))
>> return false;
>>
>> BuildMI(*FuncInfo.MBB, FuncInfo.InsertPt, DbgLoc,
>> TII.get(BranchOpc))
>> @@ -1740,7 +1740,7 @@ bool X86FastISel::X86FastEmitCMoveSelect
>>
>> EVT CmpVT = TLI.getValueType(CmpLHS->getType());
>> // Emit a compare of the LHS and RHS, setting the flags.
>> - if (!X86FastEmitCompare(CmpLHS, CmpRHS, CmpVT))
>> + if (!X86FastEmitCompare(CmpLHS, CmpRHS, CmpVT, CI->getDebugLoc()))
>> return false;
>>
>> if (SETFOpc) {
>> @@ -1924,7 +1924,7 @@ bool X86FastISel::X86FastEmitPseudoSelec
>> std::swap(CmpLHS, CmpRHS);
>>
>> EVT CmpVT = TLI.getValueType(CmpLHS->getType());
>> - if (!X86FastEmitCompare(CmpLHS, CmpRHS, CmpVT))
>> + if (!X86FastEmitCompare(CmpLHS, CmpRHS, CmpVT, CI->getDebugLoc()))
>> return false;
>> } else {
>> unsigned CondReg = getRegForValue(Cond);
>>
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150129/fe212a77/attachment.html>
More information about the llvm-commits
mailing list