[clang] [llvm] [MC] Remove UseAssemblerInfoForParsing (PR #91082)

2024-05-15 Thread Nikita Popov via cfe-commits

nikic wrote:

Reverted in 
https://github.com/llvm/llvm-project/commit/fa750f09be6966de7423ddce1af7d1eaf817182c
 due to large compile-time regressions in some cases, see 
https://llvm-compile-time-tracker.com/compare.php?from=9bbefb7f600019c9d7025281132dd160729bfff2&to=03c53c69a367008da689f0d2940e2197eb4a955c&stat=instructions:u.
 sqlite3 regresses by 5% in unoptimized builds.

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


[clang] [llvm] [MC] Remove UseAssemblerInfoForParsing (PR #91082)

2024-05-15 Thread Krzysztof Parzyszek via cfe-commits

kparzysz wrote:

Problem solved in 29c2475f215110d9e6b3955d5eb2832b3f719c2f.

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


[clang] [llvm] [MC] Remove UseAssemblerInfoForParsing (PR #91082)

2024-05-15 Thread Krzysztof Parzyszek via cfe-commits

kparzysz wrote:

> But why? I don't know what business MLIR could possibly have touching this, 
> for AMDGPU of all things

I don't know why, but my build is failing now.

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


[clang] [llvm] [MC] Remove UseAssemblerInfoForParsing (PR #91082)

2024-05-15 Thread Matt Arsenault via cfe-commits

arsenm wrote:

> It's still used:
> 
> ```
> /work/kparzysz/git/llvm.org/mlir/lib/Target/LLVM/ROCDL/Target.cpp: In member 
> function ‘std::optional > 
> mlir::ROCDL::SerializeGPUModuleBase::assembleIsa(llvm::StringRef)’:
> /work/kparzysz/git/llvm.org/mlir/lib/Target/LLVM/ROCDL/Target.cpp:302:15: 
> error:
>  ‘class llvm::MCStreamer’ has no member named ‘setUseAssemblerInfoForParsing’
>   302 |   mcStreamer->setUseAssemblerInfoForParsing(true);
>   |   ^
> ```

But why? I don't know what business MLIR could possibly have touching this, for 
AMDGPU of all things

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


[clang] [llvm] [MC] Remove UseAssemblerInfoForParsing (PR #91082)

2024-05-15 Thread Krzysztof Parzyszek via cfe-commits

kparzysz wrote:

It's still used:
```
/work/kparzysz/git/llvm.org/mlir/lib/Target/LLVM/ROCDL/Target.cpp: In member 
function ‘std::optional > 
mlir::ROCDL::SerializeGPUModuleBase::assembleIsa(llvm::StringRef)’:
/work/kparzysz/git/llvm.org/mlir/lib/Target/LLVM/ROCDL/Target.cpp:302:15: error:
 ‘class llvm::MCStreamer’ has no member named ‘setUseAssemblerInfoForParsing’
  302 |   mcStreamer->setUseAssemblerInfoForParsing(true);
  |   ^
```

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


[clang] [llvm] [MC] Remove UseAssemblerInfoForParsing (PR #91082)

2024-05-15 Thread Fangrui Song via cfe-commits

https://github.com/MaskRay closed 
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


[clang] [llvm] [MC] Remove UseAssemblerInfoForParsing (PR #91082)

2024-05-15 Thread Fangrui Song via cfe-commits

https://github.com/MaskRay edited 
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


[clang] [llvm] [MC] Remove UseAssemblerInfoForParsing (PR #91082)

2024-05-15 Thread Fangrui Song via cfe-commits

https://github.com/MaskRay updated 
https://github.com/llvm/llvm-project/pull/91082

>From 7e36c4d9f736b9b2eff29e565d22e01c29744c1e Mon Sep 17 00:00:00 2001
From: Fangrui Song 
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 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 Parser(
diff --git a/llvm/include/llvm/MC/MCStreamer.h 
b/llvm/include/llvm/MC/MCStreamer.h
index 69867620e1bf8..50986e6bde886 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 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 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 d2da5d0d3f90f..a9003a164b306 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 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());
 }
 
diff --git a/llvm/lib/Target/AMDGPU/AMDGPUAsmPrinter.cpp 
b/llvm/lib/Target/AMDGPU/AMDGPUAsmPrinter.c

[clang] [llvm] [MC] Remove UseAssemblerInfoForParsing (PR #91082)

2024-05-15 Thread Peter Smith via cfe-commits

https://github.com/smithp35 approved this pull request.

I think it is reasonable to proceed given the RFC and the response.

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


[clang] [llvm] [MC] Remove UseAssemblerInfoForParsing (PR #91082)

2024-05-14 Thread Fangrui Song via cfe-commits

MaskRay wrote:

> > Thanks for the additional context. My main concern is that we're undoing 
> > the consensus of [reviews.llvm.org/D45164](https://reviews.llvm.org/D45164) 
> > which if I've understood the comments properly was "There is a reasonable 
> > expectation that compiled (not assembled) code should be identical, or at 
> > least as close to the assembly output.
> > I'm not hugely concerned about that personally as I don't think there are 
> > any written guarantees and I come from a background of a toolchain that 
> > didn't come close to that (assembler output was disassembled from object 
> > file), however there were some strong opinions on the original change.
> > Do we have any strong opinions from the other reviewers?
> > If there is a RFC I suggest that it would be entitled something like "[RFC] 
> > Clang assembly/object equivalence for files with inline assembly". If it is 
> > worded in such a way that this is needed for the kernel and we want to 
> > check for community input then if there is no response then we can go ahead.
> 
> Thanks for the suggestion. I created 
> [discourse.llvm.org/t/rfc-clang-assembly-object-equivalence-for-files-with-inline-assembly/78841](https://discourse.llvm.org/t/rfc-clang-assembly-object-equivalence-for-files-with-inline-assembly/78841)

So far there has only been one reply from @efriedma-quic . Shall we go ahead?

> I care less about error messages than outright miscompiles.

We don't have miscompiles, just the difference in whether a `.if` construct can 
be assembled.

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


[clang] [llvm] [MC] Remove UseAssemblerInfoForParsing (PR #91082)

2024-05-07 Thread Fangrui Song via cfe-commits

MaskRay wrote:

> Thanks for the additional context. My main concern is that we're undoing the 
> consensus of [reviews.llvm.org/D45164](https://reviews.llvm.org/D45164) which 
> if I've understood the comments properly was "There is a reasonable 
> expectation that compiled (not assembled) code should be identical, or at 
> least as close to the assembly output.
> 
> I'm not hugely concerned about that personally as I don't think there are any 
> written guarantees and I come from a background of a toolchain that didn't 
> come close to that (assembler output was disassembled from object file), 
> however there were some strong opinions on the original change.
> 
> Do we have any strong opinions from the other reviewers?
> 
> If there is a RFC I suggest that it would be entitled something like "[RFC] 
> Clang assembly/object equivalence for files with inline assembly". If it is 
> worded in such a way that this is needed for the kernel and we want to check 
> for community input then if there is no response then we can go ahead.

Thanks for the suggestion. I created 
https://discourse.llvm.org/t/rfc-clang-assembly-object-equivalence-for-files-with-inline-assembly/78841

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


[clang] [llvm] [MC] Remove UseAssemblerInfoForParsing (PR #91082)

2024-05-07 Thread Peter Smith via cfe-commits

smithp35 wrote:

Thanks for the additional context. My main concern is that we're undoing the 
consensus of https://reviews.llvm.org/D45164 which if I've understood the 
comments properly was "There is a reasonable expectation that compiled (not 
assembled) code should be identical, or at least as close to the assembly 
output.

I'm not hugely concerned about that personally as I don't think there are any 
written guarantees and I come from a background of a toolchain that didn't come 
close to that (assembler output was disassembled from object file), however 
there were some strong opinions on the original change.

Do we have any strong opinions from the other reviewers?

If there is a RFC I suggest that it would be entitled something like "[RFC] 
Clang assembly/object equivalence for files with inline assembly". If it is 
worded in such a way that this is needed for the kernel and we want to check 
for community input then if there is no response then we can go ahead.

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


[clang] [llvm] [MC] Remove UseAssemblerInfoForParsing (PR #91082)

2024-05-07 Thread Fangrui Song via cfe-commits

MaskRay wrote:

This PR will address a Linux kernel need.
https://lore.kernel.org/all/CAFP8O3JkgQsH-4Lmr2W_teuvLjOCPi1htr9r3CO1O0yLyw=a...@mail.gmail.com/

The code already assembles with `clang -S -fno-integrated-as` and `gcc -c`,
and this patch is to make `clang -c` work by removeing an unnecessary 
restriction in MCObjectStreamer.

I plan on making separate, more nuanced changes to MCAsmStreamer in the future,
(https://github.com/llvm/llvm-project/issues/62520#issuecomment-2096643143),
but I believe it is not worth holding back the MCObjectStreamer improvement.

Do you think this description is clear? I can create a Discourse post, but 
given the specific nature and subtlety of integrated assemblers, there might be 
limited general interest.

BTW: `clang -c -save-temps` using an `.s` immediate file and MCAsmStreamer 
inherently has some differences with `clang -c` using MCObjectStreamer.
For example, I've fixed a debug info issue 
https://github.com/llvm/llvm-project/pull/75022
and some clangDriver stuff I forget now.


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


[clang] [llvm] [MC] Remove UseAssemblerInfoForParsing (PR #91082)

2024-05-07 Thread Peter Smith via cfe-commits

https://github.com/smithp35 commented:

The code-changes look good, what I'm less sure about is whether this is the 
right thing to do unconditionally. 

A few months ago I would likely to have said this wouldn't matter that much as 
most people don't use `-S` in there build systems so any failures are likely to 
have been of minor incovenience. However we recently encountered a customer 
using `-save-temps` on their main build, which produces assembly and then 
re-assembles it; in our case there was a problem with describing the target to 
the assembler so that step failed.

With this in mind I'm a bit reluctant to say that this wouldn't affect anyone's 
use case.

This could be worth a RFC on Discourse to see if there is wider support or just 
apathy?

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


[clang] [llvm] [MC] Remove UseAssemblerInfoForParsing (PR #91082)

2024-05-04 Thread via cfe-commits

llvmbot wrote:




@llvm/pr-subscribers-backend-spir-v

Author: Fangrui Song (MaskRay)


Changes

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
: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 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 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())
-ret

[clang] [llvm] [MC] Remove UseAssemblerInfoForParsing (PR #91082)

2024-05-04 Thread Fangrui Song via cfe-commits

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
: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 
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 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 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()