Page MenuHomePhabricator

[mips] Fix the position of the .loc prologue_end directive in cases when the generated code contains DBG_VALUE instruction.
AbandonedPublic

Authored by dsanders on Sep 5 2014, 4:26 AM.

Details

Reviewers
sstankovic
Summary

[mips] Fix the position of the .loc prologue_end directive in cases when the generated code contains DBG_VALUE instruction.

Diff Detail

Repository
rL LLVM

Event Timeline

sstankovic updated this revision to Diff 13314.Sep 5 2014, 4:26 AM
sstankovic retitled this revision from to [mips] Fix the position of the .loc prologue_end directive in cases when the generated code contains DBG_VALUE instruction..
sstankovic updated this object.
sstankovic edited the test plan for this revision. (Show Details)
sstankovic added a reviewer: dsanders.
sstankovic added subscribers: Unknown Object (MLST), petarj.

This seems to be a bit of a cut and paste conditional. I haven't had much of a chance to look, but is there something else that can be done here?

-eric

dsanders edited edge metadata.Sep 23 2014, 3:59 AM

According to findPrologueEndLoc(), the source location from the first non-DBG_VALUE instruction without the FrameSetup flag and with a known source location gets the prologue_end marker. I've had a quick look through the other targets and there seems to be two ways that targets influence this. PowerPC and NVPTX simply uses an unknown location in their prologue (via DebugLoc()), while ARM and AArch64 annotate some (but not all) instructions with the MachineInstr::FrameSetup flag. X86 seems to do both. Other targets didn't seem to do anything about it.

I think we could safely use an unknown DebugLoc() in all cases since the line number shouldn't change until the end of the prologue. Does anyone know of any benefits to using the FrameSetup flag? It seems preferable to use it so that prologue instructions have the location of the start of the function but I doubt that it has a real effect on the output.

Does anyone know of any benefits to using the FrameSetup flag? It seems preferable to use it so that prologue instructions have the location of the start of the function but I doubt that it has a real effect on the output.

It seems the only part of LLVM that uses FrameSetup flag is the function calculateDbgValueHistory that calculates the history for local variables. I did quick check by changing Mips backend to add FrameSetup flag to some (but not all) of the instructions in the prologue, and the generated debug info in .s files was as before the change.

sstankovic updated this revision to Diff 19471.Feb 6 2015, 3:30 AM
sstankovic edited edge metadata.
sstankovic set the repository for this revision to rL LLVM.

Rebased the patch. Created new method MipsInstrInfo::getDebugLoc().

This seems to be a bit of a cut and paste conditional. I haven't had much of a chance to look, but is there something else that can be done here?

I moved the logic that creates DebugLoc to a new method.

One small inline comment.

lib/Target/Mips/MipsSEISelDAGToDAG.cpp
133–134

The subtarget in MipsISelDAGToDAG is MipsSubtarget so you shouldn't need the cast.

sstankovic updated this revision to Diff 19810.Feb 12 2015, 1:11 AM

Removed the unnecessary cast.

sstankovic updated this revision to Diff 22171.Mar 18 2015, 4:12 AM

Is there anything else that needs to be done for this patch?

dsanders commandeered this revision.Sep 22 2015, 8:17 AM
dsanders edited reviewers, added: sstankovic; removed: dsanders.

Closing an old differential revision. It was resolved by another patch

dsanders abandoned this revision.Sep 22 2015, 8:18 AM