[PATCH] D38776: [codeview] Implement FPO data assembler directives
Reid Kleckner via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Wed Oct 11 09:46:17 PDT 2017
rnk marked 4 inline comments as done.
rnk added inline comments.
================
Comment at: llvm/lib/Target/X86/MCTargetDesc/X86WinCOFFTargetStreamer.cpp:263-266
+ // Otherwise, get the codeview register number and print $N.
+ default:
+ OS << '$' << MRI->getCodeViewRegNum(LLVMReg);
+ break;
----------------
majnemer wrote:
> majnemer wrote:
> > Shouldn't X86::EAX be $eax, etc?
> Woah, apparently *both* work:
>
> https://github.com/GPUOpen-Tools/CodeXL/blob/6f0aa6b42ae47028981058ccc3c505a5884d2c2e/CodeXL/Components/CpuProfiling/AMDTCpuCallstackSampling/src/StackWalker/x86/PostfixFrameEvaluator.cpp#L129
> https://github.com/GPUOpen-Tools/CodeXL/blob/6f0aa6b42ae47028981058ccc3c505a5884d2c2e/CodeXL/Components/CpuProfiling/AMDTCpuCallstackSampling/src/StackWalker/x86/PostfixFrameEvaluator.cpp#L188
>
> I guess this way is more general?
BTW, breakpad provides some other FPO data references:
https://chromium.googlesource.com/breakpad/breakpad/+/master/src/processor/stackwalker_x86.cc
I guess I'll put all the GPRs in here, but leave the CV regnum fallback. Realistically only these registers and the other CSRs (ESI, EDI, and EBX) will appear in these postfix programs.
================
Comment at: llvm/lib/Target/X86/MCTargetDesc/X86WinCOFFTargetStreamer.cpp:315
+ OS.EmitIntValue(LocalSize, 4);
+ OS.EmitIntValue(0, 4); // FIXME: ParamsSize
+ OS.EmitIntValue(0, 4); // MaxStackSize
----------------
majnemer wrote:
> How would you extend the assembler directive to support this?
I added a number to `.cv_proc` for this. Might as well do it now.
================
Comment at: llvm/lib/Target/X86/MCTargetDesc/X86WinCOFFTargetStreamer.cpp:316
+ OS.EmitIntValue(0, 4); // FIXME: ParamsSize
+ OS.EmitIntValue(0, 4); // MaxStackSize
+ OS.EmitIntValue(FrameFuncStrTabOff, 4); // FrameFunc
----------------
majnemer wrote:
> Does MSVC stick anything interesting in here? What's it for?
I've only seen zeros so far. I think you'd have to screw around with dynamic allocas to make it do something.
================
Comment at: llvm/test/MC/COFF/cv-fpo-setframe.s:48
+ # Epilogue
+ # FIXME: Get FPO data for this once we get it for DWARF.
+ addl $20, %esp
----------------
majnemer wrote:
> How would this work with this FPO format?
The FrameData records act like nested scopes, where inner ones override the outer ones. They seem to assume the epilogue is at the end of the function, and the PC ranges look like nested intervals like this:
FrameData {
RvaStart: 0x0
CodeSize: 0x3C
FrameData {
RvaStart: 0x1
CodeSize: 0x3A
FrameData {
RvaStart: 0x2
CodeSize: 0x38
FrameData {
RvaStart: 0x3
CodeSize: 0x36
FrameData {
RvaStart: 0x4
CodeSize: 0x34
The start increases by one each time as it steps over each `push`, but the size decreases by two, which uncovers the corresponding `pop`.
Obviously, this is problematic in the face of LLVM's tail duplication and mid-function epilogues, so I doubt it'll ever get done.
https://reviews.llvm.org/D38776
More information about the llvm-commits
mailing list