Skip to content

Commit

Permalink
Revert "[AArch64] Async unwind - function prologues"
Browse files Browse the repository at this point in the history 
It caused builds to assert with:

  (StackSize == 0 && "We already have the CFA offset!"),
  function generateCompactUnwindEncoding, file AArch64AsmBackend.cpp, line 624.

when targeting iOS. See comment on the code review for reproducer.

> This patch rearranges emission of CFI instructions, so the resulting
> DWARF and `.eh_frame` information is precise at every instruction.
>
> The current state is that the unwind info is emitted only after the
> function prologue. This is fine for synchronous (e.g. C++) exceptions,
> but the information is generally incorrect when the program counter is
> at an instruction in the prologue or the epilogue, for example:
>
> ```
> stp     x29, x30, [sp, #-16]!           // 16-byte Folded Spill
> mov     x29, sp
> .cfi_def_cfa w29, 16
> ...
> ```
>
> after the `stp` is executed the (initial) rule for the CFA still says
> the CFA is in the `sp`, even though it's already offset by 16 bytes
>
> A correct unwind info could look like:
> ```
> stp     x29, x30, [sp, #-16]!           // 16-byte Folded Spill
> .cfi_def_cfa_offset 16
> mov     x29, sp
> .cfi_def_cfa w29, 16
> ...
> ```
>
> Having this information precise up to an instruction is useful for
> sampling profilers that would like to get a stack backtrace. The end
> goal (towards this patch is just a step) is to have fully working
> `-fasynchronous-unwind-tables`.
>
> Reviewed By: danielkiss, MaskRay
>
> Differential Revision: https://reviews.llvm.org/D111411

This reverts commit 32e8b55.
  • Loading branch information
@zmodem
zmodem committed Mar 4, 2022
1 parent 7a605ab commit 85c53c7
Show file tree
Hide file tree
Showing 67 changed files with 595 additions and 968 deletions.
398 changes: 226 additions & 172 deletions llvm/lib/Target/AArch64/AArch64FrameLowering.cpp
View file

Large diffs are not rendered by default.

9 changes: 5 additions & 4 deletions llvm/lib/Target/AArch64/AArch64FrameLowering.h
View file
Expand Up @@ -141,12 +141,13 @@ class AArch64FrameLowering : public TargetFrameLowering {
int64_t assignSVEStackObjectOffsets(MachineFrameInfo &MF,
int &MinCSFrameIndex,
int &MaxCSFrameIndex) const;
MCCFIInstruction
createDefCFAExpressionFromSP(const TargetRegisterInfo &TRI,
const StackOffset &OffsetFromSP) const;
MCCFIInstruction createCfaOffset(const TargetRegisterInfo &MRI, unsigned DwarfReg,
const StackOffset &OffsetFromDefCFA) const;
bool shouldCombineCSRLocalStackBumpInEpilogue(MachineBasicBlock &MBB,
unsigned StackBumpBytes) const;
void emitCalleeSavedGPRLocations(MachineBasicBlock &MBB,
MachineBasicBlock::iterator MBBI) const;
void emitCalleeSavedSVELocations(MachineBasicBlock &MBB,
MachineBasicBlock::iterator MBBI) const;
};

} // End llvm namespace
Expand Down
161 changes: 5 additions & 156 deletions llvm/lib/Target/AArch64/AArch64InstrInfo.cpp
View file
Expand Up @@ -42,7 +42,6 @@
#include "llvm/Support/CommandLine.h"
#include "llvm/Support/Compiler.h"
#include "llvm/Support/ErrorHandling.h"
#include "llvm/Support/LEB128.h"
#include "llvm/Support/MathExtras.h"
#include "llvm/Target/TargetMachine.h"
#include "llvm/Target/TargetOptions.h"
Expand Down Expand Up @@ -4079,118 +4078,6 @@ void AArch64InstrInfo::decomposeStackOffsetForFrameOffsets(
}
}

// Convenience function to create a DWARF expression for
// Expr + NumBytes + NumVGScaledBytes * AArch64::VG
static void appendVGScaledOffsetExpr(SmallVectorImpl<char> &Expr, int NumBytes,
int NumVGScaledBytes, unsigned VG,
llvm::raw_string_ostream &Comment) {
uint8_t buffer[16];

if (NumBytes) {
Expr.push_back(dwarf::DW_OP_consts);
Expr.append(buffer, buffer + encodeSLEB128(NumBytes, buffer));
Expr.push_back((uint8_t)dwarf::DW_OP_plus);
Comment << (NumBytes < 0 ? " - " : " + ") << std::abs(NumBytes);
}

if (NumVGScaledBytes) {
Expr.push_back((uint8_t)dwarf::DW_OP_consts);
Expr.append(buffer, buffer + encodeSLEB128(NumVGScaledBytes, buffer));

Expr.push_back((uint8_t)dwarf::DW_OP_bregx);
Expr.append(buffer, buffer + encodeULEB128(VG, buffer));
Expr.push_back(0);

Expr.push_back((uint8_t)dwarf::DW_OP_mul);
Expr.push_back((uint8_t)dwarf::DW_OP_plus);

Comment << (NumVGScaledBytes < 0 ? " - " : " + ")
<< std::abs(NumVGScaledBytes) << " * VG";
}
}

// Creates an MCCFIInstruction:
// { DW_CFA_def_cfa_expression, ULEB128 (sizeof expr), expr }
static MCCFIInstruction createDefCFAExpression(const TargetRegisterInfo &TRI,
unsigned Reg,
const StackOffset &Offset) {
int64_t NumBytes, NumVGScaledBytes;
AArch64InstrInfo::decomposeStackOffsetForDwarfOffsets(Offset, NumBytes,
NumVGScaledBytes);
std::string CommentBuffer;
llvm::raw_string_ostream Comment(CommentBuffer);

if (Reg == AArch64::SP)
Comment << "sp";
else if (Reg == AArch64::FP)
Comment << "fp";
else
Comment << printReg(Reg, &TRI);

// Build up the expression (Reg + NumBytes + NumVGScaledBytes * AArch64::VG)
SmallString<64> Expr;
unsigned DwarfReg = TRI.getDwarfRegNum(Reg, true);
Expr.push_back((uint8_t)(dwarf::DW_OP_breg0 + DwarfReg));
Expr.push_back(0);
appendVGScaledOffsetExpr(Expr, NumBytes, NumVGScaledBytes,
TRI.getDwarfRegNum(AArch64::VG, true), Comment);

// Wrap this into DW_CFA_def_cfa.
SmallString<64> DefCfaExpr;
DefCfaExpr.push_back(dwarf::DW_CFA_def_cfa_expression);
uint8_t buffer[16];
DefCfaExpr.append(buffer, buffer + encodeULEB128(Expr.size(), buffer));
DefCfaExpr.append(Expr.str());
return MCCFIInstruction::createEscape(nullptr, DefCfaExpr.str(),
Comment.str());
}

MCCFIInstruction llvm::createDefCFA(const TargetRegisterInfo &TRI,
unsigned FrameReg, unsigned Reg,
const StackOffset &Offset) {
if (Offset.getScalable())
return createDefCFAExpression(TRI, Reg, Offset);

if (FrameReg == Reg)
return MCCFIInstruction::cfiDefCfaOffset(nullptr, int(Offset.getFixed()));

unsigned DwarfReg = TRI.getDwarfRegNum(Reg, true);
return MCCFIInstruction::cfiDefCfa(nullptr, DwarfReg, (int)Offset.getFixed());
}

MCCFIInstruction llvm::createCFAOffset(const TargetRegisterInfo &TRI,
unsigned Reg,
const StackOffset &OffsetFromDefCFA) {
int64_t NumBytes, NumVGScaledBytes;
AArch64InstrInfo::decomposeStackOffsetForDwarfOffsets(
OffsetFromDefCFA, NumBytes, NumVGScaledBytes);

unsigned DwarfReg = TRI.getDwarfRegNum(Reg, true);

// Non-scalable offsets can use DW_CFA_offset directly.
if (!NumVGScaledBytes)
return MCCFIInstruction::createOffset(nullptr, DwarfReg, NumBytes);

std::string CommentBuffer;
llvm::raw_string_ostream Comment(CommentBuffer);
Comment << printReg(Reg, &TRI) << " @ cfa";

// Build up expression (NumBytes + NumVGScaledBytes * AArch64::VG)
SmallString<64> OffsetExpr;
appendVGScaledOffsetExpr(OffsetExpr, NumBytes, NumVGScaledBytes,
TRI.getDwarfRegNum(AArch64::VG, true), Comment);

// Wrap this into DW_CFA_expression
SmallString<64> CfaExpr;
CfaExpr.push_back(dwarf::DW_CFA_expression);
uint8_t buffer[16];
CfaExpr.append(buffer, buffer + encodeULEB128(DwarfReg, buffer));
CfaExpr.append(buffer, buffer + encodeULEB128(OffsetExpr.size(), buffer));
CfaExpr.append(OffsetExpr.str());

return MCCFIInstruction::createEscape(nullptr, CfaExpr.str(), Comment.str());
}

// Helper function to emit a frame offset adjustment from a given
// pointer (SrcReg), stored into DestReg. This function is explicit
// in that it requires the opcode.
Expand All @@ -4200,8 +4087,7 @@ static void emitFrameOffsetAdj(MachineBasicBlock &MBB,
unsigned SrcReg, int64_t Offset, unsigned Opc,
const TargetInstrInfo *TII,
MachineInstr::MIFlag Flag, bool NeedsWinCFI,
bool *HasWinCFI, bool EmitCFAOffset,
StackOffset CFAOffset, unsigned FrameReg) {
bool *HasWinCFI) {
int Sign = 1;
unsigned MaxEncoding, ShiftSize;
switch (Opc) {
Expand All @@ -4226,13 +4112,6 @@ static void emitFrameOffsetAdj(MachineBasicBlock &MBB,
llvm_unreachable("Unsupported opcode");
}

// `Offset` can be in bytes or in "scalable bytes".
int VScale = 1;
if (Opc == AArch64::ADDVL_XXI)
VScale = 16;
else if (Opc == AArch64::ADDPL_XXI)
VScale = 2;

// FIXME: If the offset won't fit in 24-bits, compute the offset into a
// scratch register. If DestReg is a virtual register, use it as the
// scratch register; otherwise, create a new virtual register (to be
Expand Down Expand Up @@ -4270,26 +4149,6 @@ static void emitFrameOffsetAdj(MachineBasicBlock &MBB,
AArch64_AM::getShifterImm(AArch64_AM::LSL, LocalShiftSize));
MBI = MBI.setMIFlag(Flag);

auto Change =
VScale == 1
? StackOffset::getFixed(ThisVal << LocalShiftSize)
: StackOffset::getScalable(VScale * (ThisVal << LocalShiftSize));
if (Sign == -1 || Opc == AArch64::SUBXri || Opc == AArch64::SUBSXri)
CFAOffset += Change;
else
CFAOffset -= Change;
if (EmitCFAOffset && DestReg == TmpReg) {
MachineFunction &MF = *MBB.getParent();
const TargetSubtargetInfo &STI = MF.getSubtarget();
const TargetRegisterInfo &TRI = *STI.getRegisterInfo();

unsigned CFIIndex =
MF.addFrameInst(createDefCFA(TRI, FrameReg, DestReg, CFAOffset));
BuildMI(MBB, MBBI, DL, TII->get(TargetOpcode::CFI_INSTRUCTION))
.addCFIIndex(CFIIndex)
.setMIFlags(Flag);
}

if (NeedsWinCFI) {
assert(Sign == 1 && "SEH directives should always have a positive sign");
int Imm = (int)(ThisVal << LocalShiftSize);
Expand Down Expand Up @@ -4326,9 +4185,7 @@ void llvm::emitFrameOffset(MachineBasicBlock &MBB,
unsigned DestReg, unsigned SrcReg,
StackOffset Offset, const TargetInstrInfo *TII,
MachineInstr::MIFlag Flag, bool SetNZCV,
bool NeedsWinCFI, bool *HasWinCFI,
bool EmitCFAOffset, StackOffset CFAOffset,
unsigned FrameReg) {
bool NeedsWinCFI, bool *HasWinCFI) {
int64_t Bytes, NumPredicateVectors, NumDataVectors;
AArch64InstrInfo::decomposeStackOffsetForFrameOffsets(
Offset, Bytes, NumPredicateVectors, NumDataVectors);
Expand All @@ -4343,13 +4200,8 @@ void llvm::emitFrameOffset(MachineBasicBlock &MBB,
Opc = SetNZCV ? AArch64::SUBSXri : AArch64::SUBXri;
}
emitFrameOffsetAdj(MBB, MBBI, DL, DestReg, SrcReg, Bytes, Opc, TII, Flag,
NeedsWinCFI, HasWinCFI, EmitCFAOffset, CFAOffset,
FrameReg);
CFAOffset += (Opc == AArch64::ADDXri || Opc == AArch64::ADDSXri)
? StackOffset::getFixed(-Bytes)
: StackOffset::getFixed(Bytes);
NeedsWinCFI, HasWinCFI);
SrcReg = DestReg;
FrameReg = DestReg;
}

assert(!(SetNZCV && (NumPredicateVectors || NumDataVectors)) &&
Expand All @@ -4359,17 +4211,14 @@ void llvm::emitFrameOffset(MachineBasicBlock &MBB,

if (NumDataVectors) {
emitFrameOffsetAdj(MBB, MBBI, DL, DestReg, SrcReg, NumDataVectors,
AArch64::ADDVL_XXI, TII, Flag, NeedsWinCFI, nullptr,
EmitCFAOffset, CFAOffset, FrameReg);
CFAOffset += StackOffset::getScalable(-NumDataVectors * 16);
AArch64::ADDVL_XXI, TII, Flag, NeedsWinCFI, nullptr);
SrcReg = DestReg;
}

if (NumPredicateVectors) {
assert(DestReg != AArch64::SP && "Unaligned access to SP");
emitFrameOffsetAdj(MBB, MBBI, DL, DestReg, SrcReg, NumPredicateVectors,
AArch64::ADDPL_XXI, TII, Flag, NeedsWinCFI, nullptr,
EmitCFAOffset, CFAOffset, FrameReg);
AArch64::ADDPL_XXI, TII, Flag, NeedsWinCFI, nullptr);
}
}

Expand Down
9 changes: 1 addition & 8 deletions llvm/lib/Target/AArch64/AArch64InstrInfo.h
View file
Expand Up @@ -395,11 +395,6 @@ bool isNZCVTouchedInInstructionRange(const MachineInstr &DefMI,
const MachineInstr &UseMI,
const TargetRegisterInfo *TRI);

MCCFIInstruction createDefCFA(const TargetRegisterInfo &TRI, unsigned FrameReg,
unsigned Reg, const StackOffset &Offset);
MCCFIInstruction createCFAOffset(const TargetRegisterInfo &MRI, unsigned Reg,
const StackOffset &OffsetFromDefCFA);

/// emitFrameOffset - Emit instructions as needed to set DestReg to SrcReg
/// plus Offset. This is intended to be used from within the prolog/epilog
/// insertion (PEI) pass, where a virtual scratch register may be allocated
Expand All @@ -409,9 +404,7 @@ void emitFrameOffset(MachineBasicBlock &MBB, MachineBasicBlock::iterator MBBI,
StackOffset Offset, const TargetInstrInfo *TII,
MachineInstr::MIFlag = MachineInstr::NoFlags,
bool SetNZCV = false, bool NeedsWinCFI = false,
bool *HasWinCFI = nullptr, bool EmitCFAOffset = false,
StackOffset InitialOffset = {},
unsigned FrameReg = AArch64::SP);
bool *HasWinCFI = nullptr);

/// rewriteAArch64FrameIndex - Rewrite MI to access 'Offset' bytes from the
/// FP. Return false if the offset could not be handled directly in MI, and
Expand Down
3 changes: 1 addition & 2 deletions llvm/test/CodeGen/AArch64/GlobalISel/byval-call.ll
View file
Expand Up @@ -7,8 +7,8 @@ define void @call_byval_i32(i32* %incoming) {
; CHECK-LABEL: call_byval_i32:
; CHECK: // %bb.0:
; CHECK-NEXT: sub sp, sp, #32
; CHECK-NEXT: .cfi_def_cfa_offset 32
; CHECK-NEXT: str x30, [sp, #16] // 8-byte Folded Spill
; CHECK-NEXT: .cfi_def_cfa_offset 32
; CHECK-NEXT: .cfi_offset w30, -16
; CHECK-NEXT: ldr w8, [x0]
; CHECK-NEXT: str w8, [sp]
Expand All @@ -26,7 +26,6 @@ define void @call_byval_a64i32([64 x i32]* %incoming) {
; CHECK-LABEL: call_byval_a64i32:
; CHECK: // %bb.0:
; CHECK-NEXT: sub sp, sp, #288
; CHECK-NEXT: .cfi_def_cfa_offset 288
; CHECK-NEXT: stp x29, x30, [sp, #256] // 16-byte Folded Spill
; CHECK-NEXT: str x28, [sp, #272] // 8-byte Folded Spill
; CHECK-NEXT: add x29, sp, #256
Expand Down
4 changes: 2 additions & 2 deletions llvm/test/CodeGen/AArch64/GlobalISel/call-translator-variadic-musttail.ll
View file
Expand Up @@ -31,13 +31,13 @@ define i32 @test_musttail_variadic_spill(i32 %arg0, ...) {
; CHECK-LABEL: test_musttail_variadic_spill:
; CHECK: ; %bb.0:
; CHECK-NEXT: sub sp, sp, #224
; CHECK-NEXT: .cfi_def_cfa_offset 224
; CHECK-NEXT: stp x28, x27, [sp, #128] ; 16-byte Folded Spill
; CHECK-NEXT: stp x26, x25, [sp, #144] ; 16-byte Folded Spill
; CHECK-NEXT: stp x24, x23, [sp, #160] ; 16-byte Folded Spill
; CHECK-NEXT: stp x22, x21, [sp, #176] ; 16-byte Folded Spill
; CHECK-NEXT: stp x20, x19, [sp, #192] ; 16-byte Folded Spill
; CHECK-NEXT: stp x29, x30, [sp, #208] ; 16-byte Folded Spill
; CHECK-NEXT: .cfi_def_cfa_offset 224
; CHECK-NEXT: .cfi_offset w30, -8
; CHECK-NEXT: .cfi_offset w29, -16
; CHECK-NEXT: .cfi_offset w19, -24
Expand Down Expand Up @@ -103,13 +103,13 @@ define void @f_thunk(i8* %this, ...) {
; CHECK-LABEL: f_thunk:
; CHECK: ; %bb.0:
; CHECK-NEXT: sub sp, sp, #256
; CHECK-NEXT: .cfi_def_cfa_offset 256
; CHECK-NEXT: stp x28, x27, [sp, #160] ; 16-byte Folded Spill
; CHECK-NEXT: stp x26, x25, [sp, #176] ; 16-byte Folded Spill
; CHECK-NEXT: stp x24, x23, [sp, #192] ; 16-byte Folded Spill
; CHECK-NEXT: stp x22, x21, [sp, #208] ; 16-byte Folded Spill
; CHECK-NEXT: stp x20, x19, [sp, #224] ; 16-byte Folded Spill
; CHECK-NEXT: stp x29, x30, [sp, #240] ; 16-byte Folded Spill
; CHECK-NEXT: .cfi_def_cfa_offset 256
; CHECK-NEXT: .cfi_offset w30, -8
; CHECK-NEXT: .cfi_offset w29, -16
; CHECK-NEXT: .cfi_offset w19, -24
Expand Down

0 comments on commit 85c53c7

Please sign in to comment.