[llc] Add -M for InstPrinter options #121078

MaskRay · 2024-12-25T01:16:39Z

For many targets, llvm-objdump and llvm-mc
(https://reviews.llvm.org/D103004) support -M no-aliases (e.g.
RISCVInstPrinter::applyTargetSpecificCLOption).

This patch implements -M for llc.

While here, rename "DisassemblerOptions" in llvm-mc to the more
appropriate "InstPrinterOptions". For llvm-mc --assemble, there is no
disassembler involved.

Created using spr 1.3.5-bogner

s-barannikov

The way it is currently implemented it only works when invoking llc directly.
It would be great if the option could be set by the compiler driver, which passes options in memory.
See

llvm-project/clang/tools/driver/cc1as_main.cpp

Line 538 in eb549da

MCOptions.AsmVerbose = true;

for a few examples.

Some targets prefer to not print aliases by default; there should be a way of enabling this behavior.

llvm/lib/CodeGen/CodeGenTargetMachineImpl.cpp

MaskRay · 2024-12-25T05:15:51Z

The way it is currently implemented it only works when invoking llc directly. It would be great if the option could be set by the compiler driver, which passes options in memory. See

llvm-project/clang/tools/driver/cc1as_main.cpp

Line 538 in eb549da

MCOptions.AsmVerbose = true;

for a few examples.
Some targets prefer to not print aliases by default; there should be a way of enabling this behavior.

Thanks for the quick response. This patch intends to port applyTargetSpecificCLOption to llc to be on par with llvm-mc and llvm-objdump.

You are right that Clang cc1as sets MCTargetOptions in a different place. If we want to add a clang -cc1 and -cc1as option, the clang file needs to be modified. That isn't my motivation, though. (Skimming through GCC's manpage I don't find a similar option. If we add an option, we have some freedom on the naming.)

s-barannikov · 2024-12-25T05:29:44Z

I'm not against the current approach, it is just I'm not sure that this option would be as useful for llc without allowing it to be forwarded from the driver. llc is more a developer tool than an end-user tool. I doubt developers will want to pass this option to llc (what for?).

MaskRay · 2024-12-26T01:40:43Z

I'm not against the current approach, it is just I'm not sure that this option would be as useful for llc without allowing it to be forwarded from the driver. llc is more a developer tool than an end-user tool. I doubt developers will want to pass this option to llc (what for?).

For some reasons some RISC-V tests use a cl::opt option riscv-no-aliases. With this llc option, riscv-no-aliases/csky-no-aliases can be replaced with -M no-aliases, aligning with llvm-objdump. The usefulness is indeed low, just to unify similar features and avoid cl::opt.

Created using spr 1.3.5-bogner

MaskRay · 2025-04-08T05:59:59Z

Just realized that I haven't made the simplifications you requested :) Done. I think this is still useful, even just to remove riscv-no-aliases/csky-no-aliases and unify llc/llvm-mc/llvm-objdump

topperc

LGTM

now that llc supports `-M no-aliases` (along with llvm-mc and llvm-objdump) (#121078). Pull Request: #134879

For many targets, llvm-objdump and llvm-mc (https://reviews.llvm.org/D103004) support -M no-aliases (e.g. `RISCVInstPrinter::applyTargetSpecificCLOption`). This patch implements -M for llc. While here, rename "DisassemblerOptions" in llvm-mc to the more appropriate "InstPrinterOptions". For llvm-mc --assemble, there is no disassembler involved. Pull Request: llvm/llvm-project#121078

thurstond · 2025-04-09T04:44:02Z

Greetings from the borg! The HWASan buildbot is failing on the newly added llvm/test/tools/llc/instprinter-options.ll: https://lab.llvm.org/buildbot/#/builders/55/builds/9607

******************** TEST 'LLVM :: tools/llc/instprinter-options.ll' FAILED ********************
Exit Code: 1
Command Output (stderr):
--
not /home/b/sanitizer-aarch64-linux-bootstrap-hwasan/build/llvm_build_hwasan/bin/llc -mtriple=x86_64 < /home/b/sanitizer-aarch64-linux-bootstrap-hwasan/build/llvm-project/llvm/test/tools/llc/instprinter-options.ll -M invalid 2>&1 | /home/b/sanitizer-aarch64-linux-bootstrap-hwasan/build/llvm_build_hwasan/bin/FileCheck /home/b/sanitizer-aarch64-linux-bootstrap-hwasan/build/llvm-project/llvm/test/tools/llc/instprinter-options.ll --implicit-check-not=error: # RUN: at line 2
+ not /home/b/sanitizer-aarch64-linux-bootstrap-hwasan/build/llvm_build_hwasan/bin/llc -mtriple=x86_64 -M invalid
+ /home/b/sanitizer-aarch64-linux-bootstrap-hwasan/build/llvm_build_hwasan/bin/FileCheck /home/b/sanitizer-aarch64-linux-bootstrap-hwasan/build/llvm-project/llvm/test/tools/llc/instprinter-options.ll --implicit-check-not=error:
command line:1:22: error: IMPLICIT-CHECK-NOT: excluded string found in input
-implicit-check-not='error:'
                     ^
<stdin>:3:1: note: found here
error: Aborted
^~~~~~
Input file: <stdin>
Check file: /home/b/sanitizer-aarch64-linux-bootstrap-hwasan/build/llvm-project/llvm/test/tools/llc/instprinter-options.ll
-dump-input=help explains the following input dump.
Input was:
<<<<<<
          1: /home/b/sanitizer-aarch64-linux-bootstrap-hwasan/build/llvm_build_hwasan/bin/llc: error: invalid InstPrinter option 'invalid' 
          2: libc++abi: Pure virtual function called! 
          3: error: Aborted 
not:imp1     !~~~~~          error: no match expected
>>>>>>

... to clarify ownership, aligning with other parameters. Using `std::unique_ptr` encourages users to manage `createMCInstPrinter` with a unique_ptr instead of a raw pointer, reducing the risk of memory leaks. * llvm-mc: fix a leak and update llvm/test/tools/llvm-mc/disassembler-options.test * #121078 copied the llvm-mc code to CodeGenTargetMachineImpl and made the same mistake. Fixed by 2b8cc65 Using unique_ptr requires #include MCInstPrinter.h in a few translation units. * Delete a createAsmStreamer overload I deprecated in 2024 * SystemZMCTargetDesc.cpp: rename to `createSystemZAsmStreamer` to fix an overload conflict. Pull Request: #135128

For many targets, llvm-objdump and llvm-mc (https://reviews.llvm.org/D103004) support -M no-aliases (e.g. `RISCVInstPrinter::applyTargetSpecificCLOption`). This patch implements -M for llc. While here, rename "DisassemblerOptions" in llvm-mc to the more appropriate "InstPrinterOptions". For llvm-mc --assemble, there is no disassembler involved. Pull Request: llvm#121078

now that llc supports `-M no-aliases` (along with llvm-mc and llvm-objdump) (llvm#121078). Pull Request: llvm#134879

... to clarify ownership, aligning with other parameters. Using `std::unique_ptr` encourages users to manage `createMCInstPrinter` with a unique_ptr instead of a raw pointer, reducing the risk of memory leaks. * llvm-mc: fix a leak and update llvm/test/tools/llvm-mc/disassembler-options.test * llvm#121078 copied the llvm-mc code to CodeGenTargetMachineImpl and made the same mistake. Fixed by 2b8cc65 Using unique_ptr requires #include MCInstPrinter.h in a few translation units. * Delete a createAsmStreamer overload I deprecated in 2024 * SystemZMCTargetDesc.cpp: rename to `createSystemZAsmStreamer` to fix an overload conflict. Pull Request: llvm#135128

[𝘀𝗽𝗿] initial version

01b34a5

Created using spr 1.3.5-bogner

MaskRay requested review from topperc, s-barannikov and luismarques December 25, 2024 01:17

s-barannikov reviewed Dec 25, 2024

View reviewed changes

llvm/lib/CodeGen/CodeGenTargetMachineImpl.cpp Outdated Show resolved Hide resolved

llvm/lib/CodeGen/CodeGenTargetMachineImpl.cpp Outdated Show resolved Hide resolved

simplify

c96767b

Created using spr 1.3.5-bogner

s-barannikov approved these changes Apr 8, 2025

View reviewed changes

MaskRay mentioned this pull request Apr 8, 2025

[RISCV,test] Replace -riscv-no-aliases with -M no-aliases #134879

Merged

topperc approved these changes Apr 8, 2025

View reviewed changes

lenary approved these changes Apr 8, 2025

View reviewed changes

MaskRay merged commit 02b377d into main Apr 9, 2025
12 checks passed

MaskRay deleted the users/MaskRay/spr/llc-add-m-for-instprinter-options-1 branch April 9, 2025 02:34

MaskRay added a commit that referenced this pull request Apr 9, 2025

[RISCV,test] Replace -riscv-no-aliases with -M no-aliases

a1f1bbf

now that llc supports `-M no-aliases` (along with llvm-mc and llvm-objdump) (#121078). Pull Request: #134879

thurstond mentioned this pull request Apr 9, 2025

Revert "[llc] Add -M for InstPrinter options" #135070

Closed

MaskRay mentioned this pull request Apr 10, 2025

MCAsmStreamer: Replace the MCInstPrinter * parameter with unique_ptr #135128

Merged

var-const pushed a commit to ldionne/llvm-project that referenced this pull request Apr 17, 2025

[RISCV,test] Replace -riscv-no-aliases with -M no-aliases

bfce5e0

now that llc supports `-M no-aliases` (along with llvm-mc and llvm-objdump) (llvm#121078). Pull Request: llvm#134879

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[llc] Add -M for InstPrinter options #121078

[llc] Add -M for InstPrinter options #121078

Uh oh!

MaskRay commented Dec 25, 2024

Uh oh!

s-barannikov left a comment

Uh oh!

Uh oh!

Uh oh!

MaskRay commented Dec 25, 2024

Uh oh!

s-barannikov commented Dec 25, 2024

Uh oh!

MaskRay commented Dec 26, 2024 •

edited

Loading

Uh oh!

MaskRay commented Apr 8, 2025

Uh oh!

topperc left a comment

Uh oh!

Uh oh!

thurstond commented Apr 9, 2025

Uh oh!

Uh oh!

[llc] Add -M for InstPrinter options #121078

[llc] Add -M for InstPrinter options #121078

Uh oh!

Conversation

MaskRay commented Dec 25, 2024

Uh oh!

s-barannikov left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

MaskRay commented Dec 25, 2024

Uh oh!

s-barannikov commented Dec 25, 2024

Uh oh!

MaskRay commented Dec 26, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

MaskRay commented Apr 8, 2025

Uh oh!

topperc left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

thurstond commented Apr 9, 2025

Uh oh!

Uh oh!

MaskRay commented Dec 26, 2024 •

edited

Loading