[PATCH] D113613: [ThinLTO][MC] Use conditional assignments for promotion aliases
Peter Collingbourne via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Thu Dec 9 19:47:11 PST 2021
pcc added a comment.
I don't see why this new directive should imply `.no_dead_strip` on Mac. Indeed, the whole point of this new directive is to allow dead stripping (whether it's done by the compiler or by the linker).
I also found the multiple bools passed to the various functions very confusing and I think this needs to be cleaned up.
The patch below (on top of yours) passes all of your tests:
diff --git a/llvm/include/llvm/MC/MCObjectStreamer.h b/llvm/include/llvm/MC/MCObjectStreamer.h
index 27a2e07faab6..183fd79fb9fc 100644
--- a/llvm/include/llvm/MC/MCObjectStreamer.h
+++ b/llvm/include/llvm/MC/MCObjectStreamer.h
@@ -53,7 +53,6 @@ class MCObjectStreamer : public MCStreamer {
struct PendingAssignment {
MCSymbol *Symbol;
const MCExpr *Value;
- MCSymbolAttr Attr;
};
/// A list of conditional assignments we may need to emit if the target
@@ -129,8 +128,8 @@ public:
virtual void emitLabelAtPos(MCSymbol *Symbol, SMLoc Loc, MCFragment *F,
uint64_t Offset);
void emitAssignment(MCSymbol *Symbol, const MCExpr *Value) override;
- void emitConditionalAssignment(MCSymbol *Symbol, const MCExpr *Value,
- MCSymbolAttr Attr = MCSA_Invalid) override;
+ void emitConditionalAssignment(MCSymbol *Symbol,
+ const MCExpr *Value) override;
void emitValueImpl(const MCExpr *Value, unsigned Size,
SMLoc Loc = SMLoc()) override;
void emitULEB128Value(const MCExpr *Value) override;
diff --git a/llvm/include/llvm/MC/MCStreamer.h b/llvm/include/llvm/MC/MCStreamer.h
index c95b7517594d..2812f6b4c1f3 100644
--- a/llvm/include/llvm/MC/MCStreamer.h
+++ b/llvm/include/llvm/MC/MCStreamer.h
@@ -517,9 +517,8 @@ public:
virtual void emitAssignment(MCSymbol *Symbol, const MCExpr *Value);
/// Emit an assignment of \p Value to \p Symbol, but only if \p Value is also
- /// emitted. If \p Attr is specified, also emit the symbol attribute.
- virtual void emitConditionalAssignment(MCSymbol *Symbol, const MCExpr *Value,
- MCSymbolAttr Attr = MCSA_Invalid);
+ /// emitted.
+ virtual void emitConditionalAssignment(MCSymbol *Symbol, const MCExpr *Value);
/// Emit an weak reference from \p Alias to \p Symbol.
///
diff --git a/llvm/lib/MC/MCAsmStreamer.cpp b/llvm/lib/MC/MCAsmStreamer.cpp
index 0eeb76b12423..5be84a0a8b2e 100644
--- a/llvm/lib/MC/MCAsmStreamer.cpp
+++ b/llvm/lib/MC/MCAsmStreamer.cpp
@@ -171,8 +171,8 @@ public:
void emitThumbFunc(MCSymbol *Func) override;
void emitAssignment(MCSymbol *Symbol, const MCExpr *Value) override;
- void emitConditionalAssignment(MCSymbol *Symbol, const MCExpr *Value,
- MCSymbolAttr Attr = MCSA_Invalid) override;
+ void emitConditionalAssignment(MCSymbol *Symbol,
+ const MCExpr *Value) override;
void emitWeakReference(MCSymbol *Alias, const MCSymbol *Symbol) override;
bool emitSymbolAttribute(MCSymbol *Symbol, MCSymbolAttr Attribute) override;
@@ -673,8 +673,7 @@ void MCAsmStreamer::emitAssignment(MCSymbol *Symbol, const MCExpr *Value) {
}
void MCAsmStreamer::emitConditionalAssignment(MCSymbol *Symbol,
- const MCExpr *Value,
- MCSymbolAttr Attr) {
+ const MCExpr *Value) {
OS << ".lto_set_conditional ";
Symbol->print(OS, MAI);
OS << ", ";
diff --git a/llvm/lib/MC/MCObjectStreamer.cpp b/llvm/lib/MC/MCObjectStreamer.cpp
index d840f2aebacc..eb6612591ca4 100644
--- a/llvm/lib/MC/MCObjectStreamer.cpp
+++ b/llvm/lib/MC/MCObjectStreamer.cpp
@@ -288,11 +288,8 @@ void MCObjectStreamer::emitLabel(MCSymbol *Symbol, SMLoc Loc) {
void MCObjectStreamer::emitPendingAssignments(MCSymbol *Symbol) {
auto Assignments = pendingAssignments.find(Symbol);
if (Assignments != pendingAssignments.end()) {
- for (const auto &A : Assignments->second) {
+ for (const auto &A : Assignments->second)
emitAssignment(A.Symbol, A.Value);
- if (A.Attr != MCSA_Invalid)
- emitSymbolAttribute(A.Symbol, A.Attr);
- }
pendingAssignments.erase(Assignments);
}
@@ -372,18 +369,15 @@ void MCObjectStreamer::emitAssignment(MCSymbol *Symbol, const MCExpr *Value) {
}
void MCObjectStreamer::emitConditionalAssignment(MCSymbol *Symbol,
- const MCExpr *Value,
- MCSymbolAttr Attr) {
+ const MCExpr *Value) {
const MCSymbol *Target = &cast<MCSymbolRefExpr>(*Value).getSymbol();
// If the symbol already exists, emit the assignment. Otherwise, emit it
// later only if the symbol is also emitted.
- if (Target->isRegistered()) {
+ if (Target->isRegistered())
emitAssignment(Symbol, Value);
- if (Attr != MCSA_Invalid)
- emitSymbolAttribute(Symbol, Attr);
- } else
- pendingAssignments[Target].push_back({Symbol, Value, Attr});
+ else
+ pendingAssignments[Target].push_back({Symbol, Value});
}
bool MCObjectStreamer::mayHaveInstructions(MCSection &Sec) const {
diff --git a/llvm/lib/MC/MCParser/AsmParser.cpp b/llvm/lib/MC/MCParser/AsmParser.cpp
index 8d43cd2f8aec..0329cf43a05c 100644
--- a/llvm/lib/MC/MCParser/AsmParser.cpp
+++ b/llvm/lib/MC/MCParser/AsmParser.cpp
@@ -356,8 +356,14 @@ private:
/// return the contents from the current token up to the end or comma.
StringRef parseStringToComma();
- bool parseAssignment(StringRef Name, bool allow_redef,
- bool NoDeadStrip = false, bool Cond = false);
+ enum class AssignmentKind {
+ Set,
+ Equiv,
+ Equal,
+ LTOSetConditional,
+ };
+
+ bool parseAssignment(StringRef Name, AssignmentKind Kind);
unsigned getBinOpPrecedence(AsmToken::TokenKind K,
MCBinaryExpr::Opcode &Kind);
@@ -566,7 +572,7 @@ private:
bool parseDirectiveFill(); // ".fill"
bool parseDirectiveZero(); // ".zero"
// ".set", ".equ", ".equiv", ".lto_set_conditional"
- bool parseDirectiveSet(StringRef IDVal, bool allow_redef, bool Cond = false);
+ bool parseDirectiveSet(StringRef IDVal, AssignmentKind Kind);
bool parseDirectiveOrg(); // ".org"
// ".align{,32}", ".p2align{,w,l}"
bool parseDirectiveAlign(bool IsPow2, unsigned ValueSize);
@@ -1969,7 +1975,7 @@ bool AsmParser::parseStatement(ParseStatementInfo &Info,
// identifier '=' ... -> assignment statement
Lex();
- return parseAssignment(IDVal, true);
+ return parseAssignment(IDVal, AssignmentKind::Equal);
default: // Normal instruction or directive.
break;
@@ -2028,11 +2034,11 @@ bool AsmParser::parseStatement(ParseStatementInfo &Info,
break;
case DK_SET:
case DK_EQU:
- return parseDirectiveSet(IDVal, true);
+ return parseDirectiveSet(IDVal, AssignmentKind::Set);
case DK_EQUIV:
- return parseDirectiveSet(IDVal, false);
+ return parseDirectiveSet(IDVal, AssignmentKind::Equiv);
case DK_LTO_SET_CONDITIONAL:
- return parseDirectiveSet(IDVal, true, true);
+ return parseDirectiveSet(IDVal, AssignmentKind::LTOSetConditional);
case DK_ASCII:
return parseDirectiveAscii(IDVal, false);
case DK_ASCIZ:
@@ -2928,12 +2934,13 @@ void AsmParser::handleMacroExit() {
ActiveMacros.pop_back();
}
-bool AsmParser::parseAssignment(StringRef Name, bool allow_redef,
- bool NoDeadStrip, bool Cond) {
+bool AsmParser::parseAssignment(StringRef Name, AssignmentKind Kind) {
MCSymbol *Sym;
const MCExpr *Value;
SMLoc ExprLoc = getTok().getLoc();
- if (MCParserUtils::parseAssignmentExpression(Name, allow_redef, *this, Sym,
+ bool AllowRedef =
+ Kind == AssignmentKind::Set || Kind == AssignmentKind::Equal;
+ if (MCParserUtils::parseAssignmentExpression(Name, AllowRedef, *this, Sym,
Value))
return true;
@@ -2948,16 +2955,21 @@ bool AsmParser::parseAssignment(StringRef Name, bool allow_redef,
return false;
// Do the assignment.
- if (Cond) {
+ switch (Kind) {
+ case AssignmentKind::Equal:
+ Out.emitAssignment(Sym, Value);
+ break;
+ case AssignmentKind::Set:
+ case AssignmentKind::Equiv:
+ Out.emitAssignment(Sym, Value);
+ Out.emitSymbolAttribute(Sym, MCSA_NoDeadStrip);
+ break;
+ case AssignmentKind::LTOSetConditional:
if (Value->getKind() != MCExpr::SymbolRef)
return Error(ExprLoc, "expected identifier");
- Out.emitConditionalAssignment(
- Sym, Value, NoDeadStrip ? MCSA_NoDeadStrip : MCSA_Invalid);
- } else {
- Out.emitAssignment(Sym, Value);
- if (NoDeadStrip)
- Out.emitSymbolAttribute(Sym, MCSA_NoDeadStrip);
+ Out.emitConditionalAssignment(Sym, Value);
+ break;
}
return false;
@@ -3011,11 +3023,10 @@ bool AsmParser::parseIdentifier(StringRef &Res) {
/// ::= .equiv identifier ',' expression
/// ::= .set identifier ',' expression
/// ::= .lto_set_conditional identifier ',' expression
-bool AsmParser::parseDirectiveSet(StringRef IDVal, bool allow_redef,
- bool Cond) {
+bool AsmParser::parseDirectiveSet(StringRef IDVal, AssignmentKind Kind) {
StringRef Name;
if (check(parseIdentifier(Name), "expected identifier") || parseComma() ||
- parseAssignment(Name, allow_redef, true, Cond))
+ parseAssignment(Name, Kind))
return true;
return false;
}
diff --git a/llvm/lib/MC/MCStreamer.cpp b/llvm/lib/MC/MCStreamer.cpp
index f4e4c5f13051..3e3d6e476e8d 100644
--- a/llvm/lib/MC/MCStreamer.cpp
+++ b/llvm/lib/MC/MCStreamer.cpp
@@ -432,8 +432,7 @@ void MCStreamer::emitLabel(MCSymbol *Symbol, SMLoc Loc) {
}
void MCStreamer::emitConditionalAssignment(MCSymbol *Symbol,
- const MCExpr *Value,
- MCSymbolAttr Attr) {}
+ const MCExpr *Value) {}
void MCStreamer::emitCFISections(bool EH, bool Debug) {}
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D113613/new/
https://reviews.llvm.org/D113613
More information about the llvm-commits
mailing list