Page MenuHomePhabricator

[AMDGPU] Emit entry function CFI
AcceptedPublic

Authored by scott.linder on Mar 26 2020, 12:20 PM.

Details

Summary

Entry functions represent the end of unwinding, as they are the
outer-most frame. This implies they can only have a meaningful
definition for the CFA, which AMDGPU defines using a memory location
description with a literal private address space address. The return
address is set to undefined as a sentinel value to signal the end of
unwinding.

Diff Detail

Event Timeline

scott.linder created this revision.Mar 26 2020, 12:20 PM

git clang-format

arsenm added inline comments.Mar 26 2020, 1:29 PM
llvm/lib/Target/AMDGPU/SIFrameLowering.cpp
630

Why two levels of casting in all of these places? I also really don't like the constructor form of casts

1509–1520

I don't think this function buys you much since all of these thing are already in scope at the use.

llvm/lib/Target/AMDGPU/SIFrameLowering.h
76

Star with lowercase

asbirlea removed a subscriber: asbirlea.Mar 26 2020, 2:33 PM
scott.linder marked 3 inline comments as done.Mar 26 2020, 3:09 PM
scott.linder added inline comments.
llvm/lib/Target/AMDGPU/SIFrameLowering.cpp
630

Sorry, this is me copy-pasting from the only other use of createEscape in the code base and not thinking about it more. I'll clean all these up.

1509–1520

It is more just to cut down on the noise from having to get a CFI index and mark the instruction as FrameSetup. I'm OK inlining it everywhere, but things go from:

BuildCFI(MBB, I, DL,
           MCCFIInstruction::createUndefined(
               nullptr, MCRI->getDwarfRegNum(AMDGPU::PC_REG, false)));

to

BuildCFI(MBB, I, DL, TII->get(TargetOpcode::CFI_INSTRUCTION))
       .addCFIIndex(MF.addFrameInst(MCCFIInstruction::createUndefined(
           nullptr, MCRI->getDwarfRegNum(AMDGPU::PC_REG, false))))
       .setMIFlag(MachineInstr::FrameSetup);
llvm/lib/Target/AMDGPU/SIFrameLowering.h
76

I was trying to mimic BuildMI, which still has the wrong casing, but I'll update all of these.

Rebase and address feedback.

RamNalamothu added inline comments.May 20 2020, 1:59 AM
llvm/lib/Target/AMDGPU/SIFrameLowering.cpp
1515

Missing MIR test checking relevant CFI_INSTRUCTIONs ?

Rebase and add an MIR test

Rebase onto LLVM master

cameron.mcinally accepted this revision.Sep 22 2020, 2:05 PM

LGTM with moderate confidence. Does anyone see a reason that this shouldn't land?

This revision is now accepted and ready to land.Sep 22 2020, 2:05 PM
greened added inline comments.Sep 30 2020, 1:33 PM
llvm/lib/Target/AMDGPU/SIFrameLowering.cpp
1533

Needs a commnent explaining what this does.

Rebase onto LLVM master and address feedback

Rebase onto LLVM trunk.