Skip to content

Commit 03c53c6

Browse files
authored
[MC] Remove UseAssemblerInfoForParsing
Commit 6c0665e (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 Link: https://discourse.llvm.org/t/rfc-clang-assembly-object-equivalence-for-files-with-inline-assembly/78841 Pull Request: #91082
1 parent 9bbefb7 commit 03c53c6

File tree

10 files changed

+16
-40
lines changed

10 files changed

+16
-40
lines changed

clang/tools/driver/cc1as_main.cpp

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -576,9 +576,6 @@ static bool ExecuteAssemblerImpl(AssemblerInvocation &Opts,
576576
Str.get()->emitZeros(1);
577577
}
578578

579-
// Assembly to object compilation should leverage assembly info.
580-
Str->setUseAssemblerInfoForParsing(true);
581-
582579
bool Failed = false;
583580

584581
std::unique_ptr<MCAsmParser> Parser(

llvm/include/llvm/MC/MCStreamer.h

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -245,8 +245,6 @@ class MCStreamer {
245245
/// requires.
246246
unsigned NextWinCFIID = 0;
247247

248-
bool UseAssemblerInfoForParsing;
249-
250248
/// Is the assembler allowed to insert padding automatically? For
251249
/// correctness reasons, we sometimes need to ensure instructions aren't
252250
/// separated in unexpected ways. At the moment, this feature is only
@@ -296,11 +294,10 @@ class MCStreamer {
296294

297295
MCContext &getContext() const { return Context; }
298296

297+
// MCObjectStreamer has an MCAssembler and allows more expression folding at
298+
// parse time.
299299
virtual MCAssembler *getAssemblerPtr() { return nullptr; }
300300

301-
void setUseAssemblerInfoForParsing(bool v) { UseAssemblerInfoForParsing = v; }
302-
bool getUseAssemblerInfoForParsing() { return UseAssemblerInfoForParsing; }
303-
304301
MCTargetStreamer *getTargetStreamer() {
305302
return TargetStreamer.get();
306303
}

llvm/lib/CodeGen/AsmPrinter/AsmPrinterInlineAsm.cpp

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -102,9 +102,6 @@ void AsmPrinter::emitInlineAsm(StringRef Str, const MCSubtargetInfo &STI,
102102
std::unique_ptr<MCAsmParser> Parser(
103103
createMCAsmParser(SrcMgr, OutContext, *OutStreamer, *MAI, BufNum));
104104

105-
// Do not use assembler-level information for parsing inline assembly.
106-
OutStreamer->setUseAssemblerInfoForParsing(false);
107-
108105
// We create a new MCInstrInfo here since we might be at the module level
109106
// and not have a MachineFunction to initialize the TargetInstrInfo from and
110107
// we only need MCInstrInfo for asm parsing. We create one unconditionally

llvm/lib/MC/MCObjectStreamer.cpp

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -40,14 +40,7 @@ MCObjectStreamer::MCObjectStreamer(MCContext &Context,
4040

4141
MCObjectStreamer::~MCObjectStreamer() = default;
4242

43-
// AssemblerPtr is used for evaluation of expressions and causes
44-
// difference between asm and object outputs. Return nullptr to in
45-
// inline asm mode to limit divergence to assembly inputs.
46-
MCAssembler *MCObjectStreamer::getAssemblerPtr() {
47-
if (getUseAssemblerInfoForParsing())
48-
return Assembler.get();
49-
return nullptr;
50-
}
43+
MCAssembler *MCObjectStreamer::getAssemblerPtr() { return Assembler.get(); }
5144

5245
void MCObjectStreamer::addPendingLabel(MCSymbol* S) {
5346
MCSection *CurSection = getCurrentSectionOnly();

llvm/lib/MC/MCStreamer.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,7 @@ void MCTargetStreamer::emitAssignment(MCSymbol *Symbol, const MCExpr *Value) {}
9393

9494
MCStreamer::MCStreamer(MCContext &Ctx)
9595
: Context(Ctx), CurrentWinFrameInfo(nullptr),
96-
CurrentProcWinFrameInfoStartIndex(0), UseAssemblerInfoForParsing(false) {
96+
CurrentProcWinFrameInfoStartIndex(0) {
9797
SectionStack.push_back(std::pair<MCSectionSubPair, MCSectionSubPair>());
9898
}
9999

llvm/lib/Target/AMDGPU/AMDGPUAsmPrinter.cpp

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -517,12 +517,9 @@ bool AMDGPUAsmPrinter::runOnMachineFunction(MachineFunction &MF) {
517517

518518
DumpCodeInstEmitter = nullptr;
519519
if (STM.dumpCode()) {
520-
// For -dumpcode, get the assembler out of the streamer, even if it does
521-
// not really want to let us have it. This only works with -filetype=obj.
522-
bool SaveFlag = OutStreamer->getUseAssemblerInfoForParsing();
523-
OutStreamer->setUseAssemblerInfoForParsing(true);
520+
// For -dumpcode, get the assembler out of the streamer. This only works
521+
// with -filetype=obj.
524522
MCAssembler *Assembler = OutStreamer->getAssemblerPtr();
525-
OutStreamer->setUseAssemblerInfoForParsing(SaveFlag);
526523
if (Assembler)
527524
DumpCodeInstEmitter = Assembler->getEmitterPtr();
528525
}

llvm/lib/Target/SPIRV/SPIRVAsmPrinter.cpp

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -114,12 +114,9 @@ void SPIRVAsmPrinter::emitEndOfAsmFile(Module &M) {
114114
// Bound is an approximation that accounts for the maximum used register
115115
// number and number of generated OpLabels
116116
unsigned Bound = 2 * (ST->getBound() + 1) + NLabels;
117-
bool FlagToRestore = OutStreamer->getUseAssemblerInfoForParsing();
118-
OutStreamer->setUseAssemblerInfoForParsing(true);
119117
if (MCAssembler *Asm = OutStreamer->getAssemblerPtr())
120118
Asm->setBuildVersion(static_cast<MachO::PlatformType>(0), Major, Minor,
121119
Bound, VersionTuple(Major, Minor, 0, Bound));
122-
OutStreamer->setUseAssemblerInfoForParsing(FlagToRestore);
123120
}
124121

125122
void SPIRVAsmPrinter::emitFunctionHeader() {

llvm/test/MC/AsmParser/assembler-expressions-inlineasm.ll

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,17 @@
1-
; RUN: not llc -mtriple x86_64-unknown-linux-gnu -o %t.s -filetype=asm %s 2>&1 | FileCheck %s
2-
; RUN: not llc -mtriple x86_64-unknown-linux-gnu -o %t.o -filetype=obj %s 2>&1 | FileCheck %s
3-
4-
; Assembler-aware expression evaluation should be disabled in inline
5-
; assembly to prevent differences in behavior between object and
6-
; assembly output.
1+
; RUN: not llc -mtriple=x86_64 %s -o /dev/null 2>&1 | FileCheck %s
2+
; RUN: llc -mtriple=x86_64 -no-integrated-as < %s | FileCheck %s --check-prefix=GAS
3+
; RUN: llc -mtriple=x86_64 -filetype=obj %s -o - | llvm-objdump -d - | FileCheck %s --check-prefix=DISASM
74

5+
; GAS: nop; .if . - foo==1; nop;.endif
86

97
; CHECK: <inline asm>:1:17: error: expected absolute expression
108

9+
; DISASM: <main>:
10+
; DISASM-NEXT: nop
11+
; DISASM-NEXT: nop
12+
; DISASM-NEXT: xorl %eax, %eax
13+
; DISASM-NEXT: retq
14+
1115
define i32 @main() local_unnamed_addr {
1216
tail call void asm sideeffect "foo: nop; .if . - foo==1; nop;.endif", "~{dirflag},~{fpsr},~{flags}"()
1317
ret i32 0

llvm/tools/llvm-mc/llvm-mc.cpp

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -569,9 +569,6 @@ int main(int argc, char **argv) {
569569
Str->initSections(true, *STI);
570570
}
571571

572-
// Use Assembler information for parsing.
573-
Str->setUseAssemblerInfoForParsing(true);
574-
575572
int Res = 1;
576573
bool disassemble = false;
577574
switch (Action) {

llvm/tools/llvm-ml/llvm-ml.cpp

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -428,9 +428,6 @@ int llvm_ml_main(int Argc, char **Argv, const llvm::ToolContext &) {
428428
Str->emitAssignment(Feat00Sym, MCConstantExpr::create(Feat00Flags, Ctx));
429429
}
430430

431-
// Use Assembler information for parsing.
432-
Str->setUseAssemblerInfoForParsing(true);
433-
434431
int Res = 1;
435432
if (InputArgs.hasArg(OPT_as_lex)) {
436433
// -as-lex; Lex only, and output a stream of tokens

0 commit comments

Comments
 (0)