Author: Fangrui Song Date: 2024-05-19T23:35:15-07:00 New Revision: 9500a5d02e23f9b43294e5f662ac099f8989c0e4
URL: https://github.com/llvm/llvm-project/commit/9500a5d02e23f9b43294e5f662ac099f8989c0e4 DIFF: https://github.com/llvm/llvm-project/commit/9500a5d02e23f9b43294e5f662ac099f8989c0e4.diff LOG: [MC] Make UseAssemblerInfoForParsing mostly true Commit 6c0665e22174d474050e85ca367424f6e02476be (https://reviews.llvm.org/D45164) enabled certain constant expression evaluation for `MCObjectStreamer` at parse time (e.g. `.if` directives, see llvm/test/MC/AsmParser/assembler-expressions.s). `getUseAssemblerInfoForParsing` was added to make `clang -c` handling inline assembly similar to `MCAsmStreamer` (e.g. `llvm-mc -filetype=asm`), where such expression folding (related to `AttemptToFoldSymbolOffsetDifference`) is unavailable. I believe this is overly conservative. We can make some parse-time expression folding work for `clang -c` even if `clang -S` would still report an error, a MCAsmStreamer issue (we cannot print `.if` directives) that should not restrict the functionality of MCObjectStreamer. ``` % cat b.cc asm(R"( .pushsection .text,"ax" .globl _start; _start: ret .if . -_start == 1 ret .endif .popsection )"); % gcc -S b.cc && gcc -c b.cc % clang -S -fno-integrated-as b.cc # succeeded % clang -c b.cc # succeeded with this patch % clang -S b.cc # still failed <inline asm>:4:5: error: expected absolute expression 4 | .if . -_start == 1 | ^ 1 error generated. ``` However, removing `getUseAssemblerInfoForParsing` would make MCDwarfFrameEmitter::Emit (for .eh_frame FDE) slow (~4% compile time regression for sqlite3.c amalgamation) due to expensive `AttemptToFoldSymbolOffsetDifference`. For now, make `UseAssemblerInfoForParsing` false in MCDwarfFrameEmitter::Emit. Close #62520 Link: https://discourse.llvm.org/t/rfc-clang-assembly-object-equivalence-for-files-with-inline-assembly/78841 Pull Request: https://github.com/llvm/llvm-project/pull/91082 Added: Modified: clang/tools/driver/cc1as_main.cpp llvm/include/llvm/MC/MCStreamer.h llvm/lib/CodeGen/AsmPrinter/AsmPrinterInlineAsm.cpp llvm/lib/MC/MCDwarf.cpp llvm/lib/MC/MCObjectStreamer.cpp llvm/lib/MC/MCStreamer.cpp llvm/lib/Target/AMDGPU/AMDGPUAsmPrinter.cpp llvm/lib/Target/SPIRV/SPIRVAsmPrinter.cpp llvm/test/MC/AsmParser/assembler-expressions-inlineasm.ll llvm/tools/llvm-mc/llvm-mc.cpp llvm/tools/llvm-ml/llvm-ml.cpp Removed: ################################################################################ diff --git a/clang/tools/driver/cc1as_main.cpp b/clang/tools/driver/cc1as_main.cpp index 86afe22fac24c..4eb753a7297a9 100644 --- a/clang/tools/driver/cc1as_main.cpp +++ b/clang/tools/driver/cc1as_main.cpp @@ -576,9 +576,6 @@ static bool ExecuteAssemblerImpl(AssemblerInvocation &Opts, Str.get()->emitZeros(1); } - // Assembly to object compilation should leverage assembly info. - Str->setUseAssemblerInfoForParsing(true); - bool Failed = false; std::unique_ptr<MCAsmParser> Parser( diff --git a/llvm/include/llvm/MC/MCStreamer.h b/llvm/include/llvm/MC/MCStreamer.h index 69867620e1bf8..b7468cf70a664 100644 --- a/llvm/include/llvm/MC/MCStreamer.h +++ b/llvm/include/llvm/MC/MCStreamer.h @@ -245,7 +245,7 @@ class MCStreamer { /// requires. unsigned NextWinCFIID = 0; - bool UseAssemblerInfoForParsing; + bool UseAssemblerInfoForParsing = true; /// Is the assembler allowed to insert padding automatically? For /// correctness reasons, we sometimes need to ensure instructions aren't @@ -296,6 +296,8 @@ class MCStreamer { MCContext &getContext() const { return Context; } + // MCObjectStreamer has an MCAssembler and allows more expression folding at + // parse time. virtual MCAssembler *getAssemblerPtr() { return nullptr; } void setUseAssemblerInfoForParsing(bool v) { UseAssemblerInfoForParsing = v; } diff --git a/llvm/lib/CodeGen/AsmPrinter/AsmPrinterInlineAsm.cpp b/llvm/lib/CodeGen/AsmPrinter/AsmPrinterInlineAsm.cpp index d0ef3e5a19391..08e3c208ba4d3 100644 --- a/llvm/lib/CodeGen/AsmPrinter/AsmPrinterInlineAsm.cpp +++ b/llvm/lib/CodeGen/AsmPrinter/AsmPrinterInlineAsm.cpp @@ -102,9 +102,6 @@ void AsmPrinter::emitInlineAsm(StringRef Str, const MCSubtargetInfo &STI, std::unique_ptr<MCAsmParser> Parser( createMCAsmParser(SrcMgr, OutContext, *OutStreamer, *MAI, BufNum)); - // Do not use assembler-level information for parsing inline assembly. - OutStreamer->setUseAssemblerInfoForParsing(false); - // We create a new MCInstrInfo here since we might be at the module level // and not have a MachineFunction to initialize the TargetInstrInfo from and // we only need MCInstrInfo for asm parsing. We create one unconditionally diff --git a/llvm/lib/MC/MCDwarf.cpp b/llvm/lib/MC/MCDwarf.cpp index 2ee0c3eb27b92..aba4071e6b910 100644 --- a/llvm/lib/MC/MCDwarf.cpp +++ b/llvm/lib/MC/MCDwarf.cpp @@ -1910,6 +1910,11 @@ void MCDwarfFrameEmitter::Emit(MCObjectStreamer &Streamer, MCAsmBackend *MAB, [](const MCDwarfFrameInfo &X, const MCDwarfFrameInfo &Y) { return CIEKey(X) < CIEKey(Y); }); + // Disable AttemptToFoldSymbolOffsetDifference folding of fdeStart-cieStart + // for EmitFDE due to the the performance issue. The label diff erences will be + // evaluate at write time. + assert(Streamer.getUseAssemblerInfoForParsing()); + Streamer.setUseAssemblerInfoForParsing(false); for (auto I = FrameArrayX.begin(), E = FrameArrayX.end(); I != E;) { const MCDwarfFrameInfo &Frame = *I; ++I; @@ -1930,6 +1935,7 @@ void MCDwarfFrameEmitter::Emit(MCObjectStreamer &Streamer, MCAsmBackend *MAB, Emitter.EmitFDE(*CIEStart, Frame, I == E, *SectionStart); } + Streamer.setUseAssemblerInfoForParsing(true); } void MCDwarfFrameEmitter::encodeAdvanceLoc(MCContext &Context, diff --git a/llvm/lib/MC/MCObjectStreamer.cpp b/llvm/lib/MC/MCObjectStreamer.cpp index d2da5d0d3f90f..0ccade91677a4 100644 --- a/llvm/lib/MC/MCObjectStreamer.cpp +++ b/llvm/lib/MC/MCObjectStreamer.cpp @@ -40,9 +40,6 @@ MCObjectStreamer::MCObjectStreamer(MCContext &Context, MCObjectStreamer::~MCObjectStreamer() = default; -// AssemblerPtr is used for evaluation of expressions and causes -// diff erence between asm and object outputs. Return nullptr to in -// inline asm mode to limit divergence to assembly inputs. MCAssembler *MCObjectStreamer::getAssemblerPtr() { if (getUseAssemblerInfoForParsing()) return Assembler.get(); diff --git a/llvm/lib/MC/MCStreamer.cpp b/llvm/lib/MC/MCStreamer.cpp index 176d55aa890be..199d865ea3496 100644 --- a/llvm/lib/MC/MCStreamer.cpp +++ b/llvm/lib/MC/MCStreamer.cpp @@ -93,7 +93,7 @@ void MCTargetStreamer::emitAssignment(MCSymbol *Symbol, const MCExpr *Value) {} MCStreamer::MCStreamer(MCContext &Ctx) : Context(Ctx), CurrentWinFrameInfo(nullptr), - CurrentProcWinFrameInfoStartIndex(0), UseAssemblerInfoForParsing(false) { + CurrentProcWinFrameInfoStartIndex(0) { SectionStack.push_back(std::pair<MCSectionSubPair, MCSectionSubPair>()); } diff --git a/llvm/lib/Target/AMDGPU/AMDGPUAsmPrinter.cpp b/llvm/lib/Target/AMDGPU/AMDGPUAsmPrinter.cpp index b7388ed9e85a8..bd48a5f80c828 100644 --- a/llvm/lib/Target/AMDGPU/AMDGPUAsmPrinter.cpp +++ b/llvm/lib/Target/AMDGPU/AMDGPUAsmPrinter.cpp @@ -517,12 +517,9 @@ bool AMDGPUAsmPrinter::runOnMachineFunction(MachineFunction &MF) { DumpCodeInstEmitter = nullptr; if (STM.dumpCode()) { - // For -dumpcode, get the assembler out of the streamer, even if it does - // not really want to let us have it. This only works with -filetype=obj. - bool SaveFlag = OutStreamer->getUseAssemblerInfoForParsing(); - OutStreamer->setUseAssemblerInfoForParsing(true); + // For -dumpcode, get the assembler out of the streamer. This only works + // with -filetype=obj. MCAssembler *Assembler = OutStreamer->getAssemblerPtr(); - OutStreamer->setUseAssemblerInfoForParsing(SaveFlag); if (Assembler) DumpCodeInstEmitter = Assembler->getEmitterPtr(); } diff --git a/llvm/lib/Target/SPIRV/SPIRVAsmPrinter.cpp b/llvm/lib/Target/SPIRV/SPIRVAsmPrinter.cpp index 2ebe5bdc47715..ad01580860448 100644 --- a/llvm/lib/Target/SPIRV/SPIRVAsmPrinter.cpp +++ b/llvm/lib/Target/SPIRV/SPIRVAsmPrinter.cpp @@ -114,12 +114,9 @@ void SPIRVAsmPrinter::emitEndOfAsmFile(Module &M) { // Bound is an approximation that accounts for the maximum used register // number and number of generated OpLabels unsigned Bound = 2 * (ST->getBound() + 1) + NLabels; - bool FlagToRestore = OutStreamer->getUseAssemblerInfoForParsing(); - OutStreamer->setUseAssemblerInfoForParsing(true); if (MCAssembler *Asm = OutStreamer->getAssemblerPtr()) Asm->setBuildVersion(static_cast<MachO::PlatformType>(0), Major, Minor, Bound, VersionTuple(Major, Minor, 0, Bound)); - OutStreamer->setUseAssemblerInfoForParsing(FlagToRestore); } void SPIRVAsmPrinter::emitFunctionHeader() { diff --git a/llvm/test/MC/AsmParser/assembler-expressions-inlineasm.ll b/llvm/test/MC/AsmParser/assembler-expressions-inlineasm.ll index 35f110f37e2fb..9d9a38f5b5a54 100644 --- a/llvm/test/MC/AsmParser/assembler-expressions-inlineasm.ll +++ b/llvm/test/MC/AsmParser/assembler-expressions-inlineasm.ll @@ -1,13 +1,17 @@ -; RUN: not llc -mtriple x86_64-unknown-linux-gnu -o %t.s -filetype=asm %s 2>&1 | FileCheck %s -; RUN: not llc -mtriple x86_64-unknown-linux-gnu -o %t.o -filetype=obj %s 2>&1 | FileCheck %s - -; Assembler-aware expression evaluation should be disabled in inline -; assembly to prevent diff erences in behavior between object and -; assembly output. +; RUN: not llc -mtriple=x86_64 %s -o /dev/null 2>&1 | FileCheck %s +; RUN: llc -mtriple=x86_64 -no-integrated-as < %s | FileCheck %s --check-prefix=GAS +; RUN: llc -mtriple=x86_64 -filetype=obj %s -o - | llvm-objdump -d - | FileCheck %s --check-prefix=DISASM +; GAS: nop; .if . - foo==1; nop;.endif ; CHECK: <inline asm>:1:17: error: expected absolute expression +; DISASM: <main>: +; DISASM-NEXT: nop +; DISASM-NEXT: nop +; DISASM-NEXT: xorl %eax, %eax +; DISASM-NEXT: retq + define i32 @main() local_unnamed_addr { tail call void asm sideeffect "foo: nop; .if . - foo==1; nop;.endif", "~{dirflag},~{fpsr},~{flags}"() ret i32 0 diff --git a/llvm/tools/llvm-mc/llvm-mc.cpp b/llvm/tools/llvm-mc/llvm-mc.cpp index 807071a7b9a16..506e4f22ef8f5 100644 --- a/llvm/tools/llvm-mc/llvm-mc.cpp +++ b/llvm/tools/llvm-mc/llvm-mc.cpp @@ -569,9 +569,6 @@ int main(int argc, char **argv) { Str->initSections(true, *STI); } - // Use Assembler information for parsing. - Str->setUseAssemblerInfoForParsing(true); - int Res = 1; bool disassemble = false; switch (Action) { diff --git a/llvm/tools/llvm-ml/llvm-ml.cpp b/llvm/tools/llvm-ml/llvm-ml.cpp index 1cac576f54e77..f1f39af059aa4 100644 --- a/llvm/tools/llvm-ml/llvm-ml.cpp +++ b/llvm/tools/llvm-ml/llvm-ml.cpp @@ -428,9 +428,6 @@ int llvm_ml_main(int Argc, char **Argv, const llvm::ToolContext &) { Str->emitAssignment(Feat00Sym, MCConstantExpr::create(Feat00Flags, Ctx)); } - // Use Assembler information for parsing. - Str->setUseAssemblerInfoForParsing(true); - int Res = 1; if (InputArgs.hasArg(OPT_as_lex)) { // -as-lex; Lex only, and output a stream of tokens _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
