[clang] [llvm] [MC] Remove UseAssemblerInfoForParsing (PR #91082)
Fangrui Song via llvm-commits
llvm-commits at lists.llvm.org
Sat May 4 13:10:32 PDT 2024
https://github.com/MaskRay created https://github.com/llvm/llvm-project/pull/91082
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
>From 7e36c4d9f736b9b2eff29e565d22e01c29744c1e Mon Sep 17 00:00:00 2001
From: Fangrui Song <i at maskray.me>
Date: Sat, 4 May 2024 13:10:21 -0700
Subject: [PATCH] =?UTF-8?q?[=F0=9D=98=80=F0=9D=97=BD=F0=9D=97=BF]=20initia?=
=?UTF-8?q?l=20version?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Created using spr 1.3.5-bogner
---
clang/tools/driver/cc1as_main.cpp | 3 ---
llvm/include/llvm/MC/MCStreamer.h | 7 ++-----
.../CodeGen/AsmPrinter/AsmPrinterInlineAsm.cpp | 3 ---
llvm/lib/MC/MCObjectStreamer.cpp | 9 +--------
llvm/lib/MC/MCStreamer.cpp | 2 +-
llvm/lib/Target/AMDGPU/AMDGPUAsmPrinter.cpp | 7 ++-----
llvm/lib/Target/SPIRV/SPIRVAsmPrinter.cpp | 3 ---
.../AsmParser/assembler-expressions-inlineasm.ll | 16 ++++++++++------
llvm/tools/llvm-mc/llvm-mc.cpp | 3 ---
llvm/tools/llvm-ml/llvm-ml.cpp | 3 ---
10 files changed, 16 insertions(+), 40 deletions(-)
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
More information about the llvm-commits
mailing list