[llvm] 9500a5d - [MC] Make UseAssemblerInfoForParsing mostly true
Fangrui Song via llvm-commits
llvm-commits at lists.llvm.org
Sun May 19 23:35:19 PDT 2024
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
More information about the llvm-commits
mailing list