Page MenuHomePhabricator

[WIP] MSVC FPO debug information
Needs ReviewPublic

Authored by jrmuizel on Jun 14 2015, 6:17 AM.

Details

Reviewers
majnemer
rnk
Summary

The basic approach is to add a FPO_INSTRUCTION during X86FrameLowering along
with some information in MachineModuleInfo. WinCodeViewLineTables then looks
for this instruction and pulls out the relevant information and emits the FPO
tables.

Diff Detail

Event Timeline

jrmuizel updated this revision to Diff 27644.Jun 14 2015, 6:17 AM
jrmuizel retitled this revision from to [WIP] MSVC FPO debug information.
jrmuizel updated this object.
jrmuizel edited the test plan for this revision. (Show Details)
jrmuizel added a reviewer: majnemer.
jrmuizel added a subscriber: Unknown Object (MLST).
rnk added a reviewer: rnk.Jun 17 2015, 1:30 PM
majnemer added inline comments.Jun 17 2015, 1:35 PM
include/llvm/CodeGen/MachineModuleInfo.h
51–53

Variable names should be upper case. Also, why not let the constructor take a const Twine & instead of SmallString?

include/llvm/Target/TargetOpcodes.h
125

FPO_INSTRUCTION is assigned the same value as STATEPOINT. Is this intentional?

lib/CodeGen/AsmPrinter/WinCodeViewLineTables.cpp
81–82

The curly bracne shouldn't be on its own line.

193–194

Please clang-format this.

197–222

These comments are very hard for me to parse. Can you have a single block comment?

226–227

Please clang-format this.

241–256

Variables start with a capital letter. Also, please avoid recalculating the end iterator on each iteration.

288

This change seems unrelated.

lib/CodeGen/AsmPrinter/WinCodeViewLineTables.h
41

Please have a space before the * token.

115

Please clang-format this line.

lib/MC/MCStreamer.cpp
23

Why did you have to make this change?

lib/Target/X86/X86FrameLowering.cpp
586

You never reassign to this variable.

1108

You never reassign to this variable.

rnk edited edge metadata.Jun 17 2015, 2:04 PM

I think you want to start by building this bottom up instead of starting all the way up at X86FrameLowering.

I'd take these steps:

  1. Build out the MC streamer support for the masm .fpo directive. This seems to handle everything we need.
  2. Take the assembler output of llc for about three interesting prologue types, manually annotate it with FPO directives, assemble it, and verify that it's debuggable.
  3. Add X86 opcodes for these directives and move them through the AsmPrinter, similar to SEH_Prologue, pushreg, etc.
aaboud added a subscriber: aaboud.Jun 18 2015, 4:05 AM
rnk added a comment.Oct 10 2017, 6:44 PM

I implemented some FPO support here: D38776 I went a different way to try to make the assembly look nice at the cost of some flexibility. My goal wasn't to support the full generality of all possible FPO data, it was more to support realistic 32-bit x86 prologues, which tend to push one of the four CSRs at constant offsets from the initial SP value.