[llvm] Reapply "[MC] Use a variant to hold MCCFIInstruction state (NFC)" (PR #170342)
Scott Linder via llvm-commits
llvm-commits at lists.llvm.org
Tue Dec 2 09:51:52 PST 2025
https://github.com/slinder1 created https://github.com/llvm/llvm-project/pull/170342
This reverts commit 8217c6415ab76c2a0f06705100c76207cd1e6bc0.
I asked Claude Sonnet 4.5 to hazard a guess at what was causing the buildbot failure, and it seems to have correctly pinned it on a bug in GCC7, although it didn't give a working fix.
I worked out of a `gcc:7.4` docker container and moved code around until `MCDwarf.cpp.o` compiled without errors or warnings, so hopefully that's sufficient to fix the bot.
Is there an easy way to do a build before landing to confirm, @Kewen12 / @ronlieb ?
>From 6efec88575a72dab72c49261d5487e59200f80d3 Mon Sep 17 00:00:00 2001
From: Scott Linder <Scott.Linder at amd.com>
Date: Tue, 2 Dec 2025 10:08:53 -0500
Subject: [PATCH] Reapply "[MC] Use a variant to hold MCCFIInstruction state
(NFC)"
This reverts commit 8217c6415ab76c2a0f06705100c76207cd1e6bc0.
---
llvm/include/llvm/MC/MCDwarf.h | 148 ++++++++++++++++-----------------
1 file changed, 71 insertions(+), 77 deletions(-)
diff --git a/llvm/include/llvm/MC/MCDwarf.h b/llvm/include/llvm/MC/MCDwarf.h
index 9944a9a92ab1f..ddfac483c73e7 100644
--- a/llvm/include/llvm/MC/MCDwarf.h
+++ b/llvm/include/llvm/MC/MCDwarf.h
@@ -29,6 +29,7 @@
#include <optional>
#include <string>
#include <utility>
+#include <variant>
#include <vector>
namespace llvm {
@@ -531,67 +532,58 @@ class MCCFIInstruction {
OpValOffset,
};
+ // Held in ExtraFields for most common OpTypes, exceptions follow.
+ struct CommonFields {
+ unsigned Register;
+ int64_t Offset;
+ unsigned Register2;
+ unsigned AddressSpace;
+ // FIXME: Workaround for GCC7 bug with nested class used as std::variant
+ // alternative where the compiler really wants a user-defined default
+ // constructor. Once we no longer support GCC7 these constructors can be
+ // replaced with default member initializers and aggregate initialization.
+ CommonFields(unsigned Reg,
+ int64_t Off = 0,
+ unsigned Reg2 = std::numeric_limits<unsigned>::max(),
+ unsigned AddrSpace = 0)
+ : Register(Reg), Offset(Off), Register2(Reg2), AddressSpace(AddrSpace) {
+ }
+ CommonFields() : CommonFields(std::numeric_limits<unsigned>::max()) {}
+ };
+ // Held in ExtraFields when OpEscape.
+ struct EscapeFields {
+ std::vector<char> Values;
+ std::string Comment;
+ };
+ // Held in ExtraFields when OpLabel.
+ struct LabelFields {
+ MCSymbol *CfiLabel = nullptr;
+ };
+
private:
MCSymbol *Label;
- union {
- struct {
- unsigned Register;
- int64_t Offset;
- } RI;
- struct {
- unsigned Register;
- int64_t Offset;
- unsigned AddressSpace;
- } RIA;
- struct {
- unsigned Register;
- unsigned Register2;
- } RR;
- MCSymbol *CfiLabel;
- } U;
+ std::variant<CommonFields, EscapeFields, LabelFields> ExtraFields;
OpType Operation;
SMLoc Loc;
- std::vector<char> Values;
- std::string Comment;
- MCCFIInstruction(OpType Op, MCSymbol *L, unsigned R, int64_t O, SMLoc Loc,
- StringRef V = "", StringRef Comment = "")
- : Label(L), Operation(Op), Loc(Loc), Values(V.begin(), V.end()),
- Comment(Comment) {
- assert(Op != OpRegister && Op != OpLLVMDefAspaceCfa);
- U.RI = {R, O};
- }
- MCCFIInstruction(OpType Op, MCSymbol *L, unsigned R1, unsigned R2, SMLoc Loc)
- : Label(L), Operation(Op), Loc(Loc) {
- assert(Op == OpRegister);
- U.RR = {R1, R2};
- }
- MCCFIInstruction(OpType Op, MCSymbol *L, unsigned R, int64_t O, unsigned AS,
- SMLoc Loc)
- : Label(L), Operation(Op), Loc(Loc) {
- assert(Op == OpLLVMDefAspaceCfa);
- U.RIA = {R, O, AS};
- }
-
- MCCFIInstruction(OpType Op, MCSymbol *L, MCSymbol *CfiLabel, SMLoc Loc)
- : Label(L), Operation(Op), Loc(Loc) {
- assert(Op == OpLabel);
- U.CfiLabel = CfiLabel;
- }
+ template <class FieldsType>
+ MCCFIInstruction(OpType Op, MCSymbol *L, FieldsType &&EF, SMLoc Loc)
+ : Label(L), ExtraFields(std::forward<FieldsType>(EF)), Operation(Op),
+ Loc(Loc) {}
public:
/// .cfi_def_cfa defines a rule for computing CFA as: take address from
/// Register and add Offset to it.
static MCCFIInstruction cfiDefCfa(MCSymbol *L, unsigned Register,
int64_t Offset, SMLoc Loc = {}) {
- return MCCFIInstruction(OpDefCfa, L, Register, Offset, Loc);
+ return {OpDefCfa, L, CommonFields{Register, Offset}, Loc};
}
/// .cfi_def_cfa_register modifies a rule for computing CFA. From now
/// on Register will be used instead of the old one. Offset remains the same.
static MCCFIInstruction createDefCfaRegister(MCSymbol *L, unsigned Register,
SMLoc Loc = {}) {
- return MCCFIInstruction(OpDefCfaRegister, L, Register, INT64_C(0), Loc);
+ return {OpDefCfaRegister, L, CommonFields{Register}, Loc};
}
/// .cfi_def_cfa_offset modifies a rule for computing CFA. Register
@@ -599,7 +591,7 @@ class MCCFIInstruction {
/// that will be added to a defined register to the compute CFA address.
static MCCFIInstruction cfiDefCfaOffset(MCSymbol *L, int64_t Offset,
SMLoc Loc = {}) {
- return MCCFIInstruction(OpDefCfaOffset, L, 0, Offset, Loc);
+ return {OpDefCfaOffset, L, CommonFields{0, Offset}, Loc};
}
/// .cfi_adjust_cfa_offset Same as .cfi_def_cfa_offset, but
@@ -607,7 +599,7 @@ class MCCFIInstruction {
/// offset.
static MCCFIInstruction createAdjustCfaOffset(MCSymbol *L, int64_t Adjustment,
SMLoc Loc = {}) {
- return MCCFIInstruction(OpAdjustCfaOffset, L, 0, Adjustment, Loc);
+ return {OpAdjustCfaOffset, L, CommonFields{0, Adjustment}, Loc};
}
// FIXME: Update the remaining docs to use the new proposal wording.
@@ -618,15 +610,15 @@ class MCCFIInstruction {
int64_t Offset,
unsigned AddressSpace,
SMLoc Loc) {
- return MCCFIInstruction(OpLLVMDefAspaceCfa, L, Register, Offset,
- AddressSpace, Loc);
+ return {OpLLVMDefAspaceCfa, L,
+ CommonFields{Register, Offset, 0, AddressSpace}, Loc};
}
/// .cfi_offset Previous value of Register is saved at offset Offset
/// from CFA.
static MCCFIInstruction createOffset(MCSymbol *L, unsigned Register,
int64_t Offset, SMLoc Loc = {}) {
- return MCCFIInstruction(OpOffset, L, Register, Offset, Loc);
+ return {OpOffset, L, CommonFields{Register, Offset}, Loc};
}
/// .cfi_rel_offset Previous value of Register is saved at offset
@@ -634,30 +626,30 @@ class MCCFIInstruction {
/// using the known displacement of the CFA register from the CFA.
static MCCFIInstruction createRelOffset(MCSymbol *L, unsigned Register,
int64_t Offset, SMLoc Loc = {}) {
- return MCCFIInstruction(OpRelOffset, L, Register, Offset, Loc);
+ return {OpRelOffset, L, CommonFields{Register, Offset}, Loc};
}
/// .cfi_register Previous value of Register1 is saved in
/// register Register2.
static MCCFIInstruction createRegister(MCSymbol *L, unsigned Register1,
unsigned Register2, SMLoc Loc = {}) {
- return MCCFIInstruction(OpRegister, L, Register1, Register2, Loc);
+ return {OpRegister, L, CommonFields{Register1, 0, Register2}, Loc};
}
/// .cfi_window_save SPARC register window is saved.
static MCCFIInstruction createWindowSave(MCSymbol *L, SMLoc Loc = {}) {
- return MCCFIInstruction(OpWindowSave, L, 0, INT64_C(0), Loc);
+ return {OpWindowSave, L, CommonFields{}, Loc};
}
/// .cfi_negate_ra_state AArch64 negate RA state.
static MCCFIInstruction createNegateRAState(MCSymbol *L, SMLoc Loc = {}) {
- return MCCFIInstruction(OpNegateRAState, L, 0, INT64_C(0), Loc);
+ return {OpNegateRAState, L, CommonFields{}, Loc};
}
/// .cfi_negate_ra_state_with_pc AArch64 negate RA state with PC.
static MCCFIInstruction createNegateRAStateWithPC(MCSymbol *L,
SMLoc Loc = {}) {
- return MCCFIInstruction(OpNegateRAStateWithPC, L, 0, INT64_C(0), Loc);
+ return {OpNegateRAStateWithPC, L, CommonFields{}, Loc};
}
/// .cfi_restore says that the rule for Register is now the same as it
@@ -665,104 +657,106 @@ class MCCFIInstruction {
/// by .cfi_startproc were executed.
static MCCFIInstruction createRestore(MCSymbol *L, unsigned Register,
SMLoc Loc = {}) {
- return MCCFIInstruction(OpRestore, L, Register, INT64_C(0), Loc);
+ return {OpRestore, L, CommonFields{Register}, Loc};
}
/// .cfi_undefined From now on the previous value of Register can't be
/// restored anymore.
static MCCFIInstruction createUndefined(MCSymbol *L, unsigned Register,
SMLoc Loc = {}) {
- return MCCFIInstruction(OpUndefined, L, Register, INT64_C(0), Loc);
+ return {OpUndefined, L, CommonFields{Register}, Loc};
}
/// .cfi_same_value Current value of Register is the same as in the
/// previous frame. I.e., no restoration is needed.
static MCCFIInstruction createSameValue(MCSymbol *L, unsigned Register,
SMLoc Loc = {}) {
- return MCCFIInstruction(OpSameValue, L, Register, INT64_C(0), Loc);
+ return {OpSameValue, L, CommonFields{Register}, Loc};
}
/// .cfi_remember_state Save all current rules for all registers.
static MCCFIInstruction createRememberState(MCSymbol *L, SMLoc Loc = {}) {
- return MCCFIInstruction(OpRememberState, L, 0, INT64_C(0), Loc);
+ return {OpRememberState, L, CommonFields{}, Loc};
}
/// .cfi_restore_state Restore the previously saved state.
static MCCFIInstruction createRestoreState(MCSymbol *L, SMLoc Loc = {}) {
- return MCCFIInstruction(OpRestoreState, L, 0, INT64_C(0), Loc);
+ return {OpRestoreState, L, CommonFields{}, Loc};
}
/// .cfi_escape Allows the user to add arbitrary bytes to the unwind
/// info.
static MCCFIInstruction createEscape(MCSymbol *L, StringRef Vals,
SMLoc Loc = {}, StringRef Comment = "") {
- return MCCFIInstruction(OpEscape, L, 0, 0, Loc, Vals, Comment);
+ return {OpEscape, L,
+ EscapeFields{std::vector<char>(Vals.begin(), Vals.end()),
+ Comment.str()},
+ Loc};
}
/// A special wrapper for .cfi_escape that indicates GNU_ARGS_SIZE
static MCCFIInstruction createGnuArgsSize(MCSymbol *L, int64_t Size,
SMLoc Loc = {}) {
- return MCCFIInstruction(OpGnuArgsSize, L, 0, Size, Loc);
+ return {OpGnuArgsSize, L, CommonFields{0, Size}, Loc};
}
static MCCFIInstruction createLabel(MCSymbol *L, MCSymbol *CfiLabel,
SMLoc Loc) {
- return MCCFIInstruction(OpLabel, L, CfiLabel, Loc);
+ return {OpLabel, L, LabelFields{CfiLabel}, Loc};
}
/// .cfi_val_offset Previous value of Register is offset Offset from the
/// current CFA register.
static MCCFIInstruction createValOffset(MCSymbol *L, unsigned Register,
int64_t Offset, SMLoc Loc = {}) {
- return MCCFIInstruction(OpValOffset, L, Register, Offset, Loc);
+ return {OpValOffset, L, CommonFields{Register, Offset}, Loc};
}
OpType getOperation() const { return Operation; }
MCSymbol *getLabel() const { return Label; }
unsigned getRegister() const {
- if (Operation == OpRegister)
- return U.RR.Register;
- if (Operation == OpLLVMDefAspaceCfa)
- return U.RIA.Register;
assert(Operation == OpDefCfa || Operation == OpOffset ||
Operation == OpRestore || Operation == OpUndefined ||
Operation == OpSameValue || Operation == OpDefCfaRegister ||
- Operation == OpRelOffset || Operation == OpValOffset);
- return U.RI.Register;
+ Operation == OpRelOffset || Operation == OpValOffset ||
+ Operation == OpRegister || Operation == OpLLVMDefAspaceCfa);
+ return std::get<CommonFields>(ExtraFields).Register;
}
unsigned getRegister2() const {
assert(Operation == OpRegister);
- return U.RR.Register2;
+ return std::get<CommonFields>(ExtraFields).Register2;
}
unsigned getAddressSpace() const {
assert(Operation == OpLLVMDefAspaceCfa);
- return U.RIA.AddressSpace;
+ return std::get<CommonFields>(ExtraFields).AddressSpace;
}
int64_t getOffset() const {
- if (Operation == OpLLVMDefAspaceCfa)
- return U.RIA.Offset;
assert(Operation == OpDefCfa || Operation == OpOffset ||
Operation == OpRelOffset || Operation == OpDefCfaOffset ||
Operation == OpAdjustCfaOffset || Operation == OpGnuArgsSize ||
- Operation == OpValOffset);
- return U.RI.Offset;
+ Operation == OpValOffset || Operation == OpLLVMDefAspaceCfa);
+ return std::get<CommonFields>(ExtraFields).Offset;
}
MCSymbol *getCfiLabel() const {
assert(Operation == OpLabel);
- return U.CfiLabel;
+ return std::get<LabelFields>(ExtraFields).CfiLabel;
}
StringRef getValues() const {
assert(Operation == OpEscape);
+ auto &Values = std::get<EscapeFields>(ExtraFields).Values;
return StringRef(&Values[0], Values.size());
}
- StringRef getComment() const { return Comment; }
+ StringRef getComment() const {
+ assert(Operation == OpEscape);
+ return std::get<EscapeFields>(ExtraFields).Comment;
+ }
SMLoc getLoc() const { return Loc; }
};
More information about the llvm-commits
mailing list