This is an archive of the discontinued LLVM Phabricator instance.

[AMDGPU] Emit entry function CFI
AbandonedPublic

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
394

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

1072–1083

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
78

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
394

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.

1072–1083

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
78

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
1078

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
1072

Needs a commnent explaining what this does.

Rebase onto LLVM master and address feedback

Rebase onto LLVM trunk.

Rebase onto LLVM top of the tree.

Herald added a project: Restricted Project. · View Herald TranscriptAug 22 2022, 2:07 PM

Incorporated all the downstream changes.

arsenm accepted this revision.Sep 15 2022, 4:01 PM
arsenm added inline comments.
llvm/lib/Target/AMDGPU/SIFrameLowering.cpp
1079

Should just pass in TII

scott.linder abandoned this revision.Mar 30 2023, 3:41 PM

Abandoned in favor of D147279 which avoids using most/all of the remaining user opcode