[PATCH] D113613: [ThinLTO][MC] Use conditional assignments for promotion aliases

[Sami Tolvanen via Phabricator via llvm-commits]

samitolvanen added inline comments.


================
Comment at: llvm/lib/MC/MCParser/AsmParser.cpp:2957-2958
+    Out.emitAssignment(Sym, Value);
+    if (NoDeadStrip)
+      Out.emitSymbolAttribute(Sym, MCSA_NoDeadStrip);
+  }
----------------
nickdesaulniers wrote:
> nickdesaulniers wrote:
> > should the check on `NoDeadStrip` occur regardless of `Cond`?
> I meant:
> ```
> if (Cond) {
>   if (Value->getKind() != MCExpr::SymbolRef)
>     return Error(ExprLoc, "expected identifier");
> 
>   Out.emitConditionalAssignment(Sym, Value);
> } else
>   Out.emitAssignment(Sym, Value);
> if (NoDeadStrip)
>   Out.emitSymbolAttribute(Sym, MCSA_NoDeadStrip);
> ```
> Rather than plumb the `MCSymbolAttr` through `emitConditionalAssignment` only to then call `emitSymbolAttribute` anyways.
I could be misreading something, but looking at `emitSymbolAttribute` implementations, `MCELFStreamer::emitSymbolAttribute` at least registers the symbol when emitting the attribute, which is exactly what we're trying to avoid unless the target symbol is also emitted. Is there a reason we would want to emit an attribute for a symbol we might not emit at all?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D113613/new/

https://reviews.llvm.org/D113613