llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-backend-spir-v Author: Fangrui Song (MaskRay) <details> <summary>Changes</summary> 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. ``` Close #<!-- -->62520 --- Full diff: https://github.com/llvm/llvm-project/pull/91082.diff 10 Files Affected: - (modified) clang/tools/driver/cc1as_main.cpp (-3) - (modified) llvm/include/llvm/MC/MCStreamer.h (+2-5) - (modified) llvm/lib/CodeGen/AsmPrinter/AsmPrinterInlineAsm.cpp (-3) - (modified) llvm/lib/MC/MCObjectStreamer.cpp (+1-8) - (modified) llvm/lib/MC/MCStreamer.cpp (+1-1) - (modified) llvm/lib/Target/AMDGPU/AMDGPUAsmPrinter.cpp (+2-5) - (modified) llvm/lib/Target/SPIRV/SPIRVAsmPrinter.cpp (-3) - (modified) llvm/test/MC/AsmParser/assembler-expressions-inlineasm.ll (+10-6) - (modified) llvm/tools/llvm-mc/llvm-mc.cpp (-3) - (modified) llvm/tools/llvm-ml/llvm-ml.cpp (-3) ``````````diff diff --git a/clang/tools/driver/cc1as_main.cpp b/clang/tools/driver/cc1as_main.cpp index 86afe22fac24cc..4eb753a7297a92 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 69867620e1bf8a..50986e6bde8867 100644 --- a/llvm/include/llvm/MC/MCStreamer.h +++ b/llvm/include/llvm/MC/MCStreamer.h @@ -245,8 +245,6 @@ class MCStreamer { /// requires. unsigned NextWinCFIID = 0; - bool UseAssemblerInfoForParsing; - /// Is the assembler allowed to insert padding automatically? For /// correctness reasons, we sometimes need to ensure instructions aren't /// separated in unexpected ways. At the moment, this feature is only @@ -296,11 +294,10 @@ 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; } - bool getUseAssemblerInfoForParsing() { return UseAssemblerInfoForParsing; } - MCTargetStreamer *getTargetStreamer() { return TargetStreamer.get(); } diff --git a/llvm/lib/CodeGen/AsmPrinter/AsmPrinterInlineAsm.cpp b/llvm/lib/CodeGen/AsmPrinter/AsmPrinterInlineAsm.cpp index d0ef3e5a19391c..08e3c208ba4d38 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/MCObjectStreamer.cpp b/llvm/lib/MC/MCObjectStreamer.cpp index d2da5d0d3f90f2..a9003a164b306d 100644 --- a/llvm/lib/MC/MCObjectStreamer.cpp +++ b/llvm/lib/MC/MCObjectStreamer.cpp @@ -40,14 +40,7 @@ MCObjectStreamer::MCObjectStreamer(MCContext &Context, MCObjectStreamer::~MCObjectStreamer() = default; -// AssemblerPtr is used for evaluation of expressions and causes -// difference 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(); - return nullptr; -} +MCAssembler *MCObjectStreamer::getAssemblerPtr() { return Assembler.get(); } void MCObjectStreamer::addPendingLabel(MCSymbol* S) { MCSection *CurSection = getCurrentSectionOnly(); diff --git a/llvm/lib/MC/MCStreamer.cpp b/llvm/lib/MC/MCStreamer.cpp index 176d55aa890bed..199d865ea3496d 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 c090d6133dbad4..f891eb97f358b8 100644 --- a/llvm/lib/Target/AMDGPU/AMDGPUAsmPrinter.cpp +++ b/llvm/lib/Target/AMDGPU/AMDGPUAsmPrinter.cpp @@ -511,12 +511,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 2ebe5bdc47715b..ad015808604487 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 35f110f37e2fb6..9d9a38f5b5a54b 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 differences 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 807071a7b9a16a..506e4f22ef8f54 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 1cac576f54e77f..f1f39af059aa49 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 `````````` </details> https://github.com/llvm/llvm-project/pull/91082 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
