Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

-Cllvm-args usability improvement #115638

Merged
merged 2 commits into from
Sep 8, 2023
Merged

Conversation

ldm0
Copy link
Contributor

@ldm0 ldm0 commented Sep 7, 2023

fixes: #26338
fixes: #115564

Two problems were found during playing with -Cllvm-args

  1. When llvm.link-shared is set to false in config.toml, output of rustc -C llvm-args='--help-list-hidden' doesn't contain --emit-dwarf-unwind and --emulated-tls. When it is set to true, rustc -C llvm-args='--help-list-hidden' emits --emit-dwarf-unwind, but --emulated-tls is still missing.
  2. Setting -Cllvm-args=--emit-dwarf-unwind=always doesn't take any effect, but -Cllvm-args=-machine-outliner-reruns=3 does work.

1

Adding RegisterCodeGenFlags to register codegen flags fixed the first problem. rustc -C llvm-args='--help-list-hidden' emits full codegen flags including --emit-dwarf-unwind and --emulated-tls.

2

Constructing TargetOptions from InitTargetOptionsFromCodeGenFlags in LLVMRustCreateTargetMachine fixed the second problem. The LLVMRustSetLLVMOptions calls ParseCommandLineOptions which parses given llvm-args. For options like machine-outliner-reruns, it just works, since the codegen logic directly consumes the parsing result:

machine-outliner-reruns register
machine-outliner-reruns consumption

But for flags defined in TargetOptions and MCTargetOptions to take effect, constructing them with InitTargetOptionsFromCodeGenFlags is essential, or the parsing result is just not consumed. Similar patterns can be observed in lli, llc, etc.

@rustbot
Copy link
Collaborator

rustbot commented Sep 7, 2023

r? @cuviper

(rustbot has picked a reviewer for you, use r? to override)

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Sep 7, 2023
@ldm0
Copy link
Contributor Author

ldm0 commented Sep 7, 2023

r? @nikic since #26338 (comment)

@rustbot rustbot assigned nikic and unassigned cuviper Sep 7, 2023
@nikic
Copy link
Contributor

nikic commented Sep 7, 2023

@bors r+

@bors
Copy link
Contributor

bors commented Sep 7, 2023

📌 Commit 487766c has been approved by nikic

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Sep 7, 2023
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request Sep 7, 2023
`-Cllvm-args` usability improvement

fixes: rust-lang#26338
fixes: rust-lang#115564

Two problems were found during playing with `-Cllvm-args`

1. When `llvm.link-shared` is set to `false` in `config.toml`, output of `rustc -C llvm-args='--help-list-hidden'` doesn't contain `--emit-dwarf-unwind` and `--emulated-tls`. When it is set to `true`, `rustc -C llvm-args='--help-list-hidden'` emits `--emit-dwarf-unwind`, but `--emulated-tls` is still missing.
2. Setting `-Cllvm-args=--emit-dwarf-unwind=always` doesn't take any effect, but `-Cllvm-args=-machine-outliner-reruns=3` does work.

### 1

Adding `RegisterCodeGenFlags` to register codegen flags fixed the first problem. `rustc -C llvm-args='--help-list-hidden'` emits full codegen flags including `--emit-dwarf-unwind` and `--emulated-tls`.

### 2

Constructing `TargetOptions` from `InitTargetOptionsFromCodeGenFlags` in `LLVMRustCreateTargetMachine` fixed the second problem. The `LLVMRustSetLLVMOptions` calls `ParseCommandLineOptions` which parses given `llvm-args`. For options like `machine-outliner-reruns`, it just works, since the codegen logic directly consumes the parsing result:

[machine-outliner-reruns register](https://github.com/rust-lang/llvm-project/blob/0537f6354cffe546cbf47f6dc9c7f82e49e86cfb/llvm/lib/CodeGen/MachineOutliner.cpp#L114)
[machine-outliner-reruns consumption](https://github.com/rust-lang/llvm-project/blob/0537f6354cffe546cbf47f6dc9c7f82e49e86cfb/llvm/lib/CodeGen/MachineOutliner.cpp#L1138)

But for flags defined in `TargetOptions` and `MCTargetOptions` to take effect, constructing them with `InitTargetOptionsFromCodeGenFlags` is essential, or the parsing result is just not consumed. Similar patterns can be observed in [lli](https://github.com/rust-lang/llvm-project/blob/0537f6354cffe546cbf47f6dc9c7f82e49e86cfb/llvm/tools/llc/llc.cpp#L494), [llc](https://github.com/rust-lang/llvm-project/blob/0537f6354cffe546cbf47f6dc9c7f82e49e86cfb/llvm/tools/lli/lli.cpp#L517), etc.
bors added a commit to rust-lang-ci/rust that referenced this pull request Sep 7, 2023
…llaumeGomez

Rollup of 8 pull requests

Successful merges:

 - rust-lang#115345 (MCP661: Move wasm32-wasi-preview1-threads target to Tier 2)
 - rust-lang#115604 (rustdoc: Render private fields in tuple struct as `/* private fields */`)
 - rust-lang#115624 (Print the path of a return-position impl trait in trait when `return_type_notation` is enabled)
 - rust-lang#115629 (Don't suggest dereferencing to unsized type)
 - rust-lang#115633 (Lint node for `PRIVATE_BOUNDS`/`PRIVATE_INTERFACES` is the item which names the private type)
 - rust-lang#115634 (Use `newtype_index` for `IntVid` and `FloatVid`.)
 - rust-lang#115638 (`-Cllvm-args` usability improvement)
 - rust-lang#115649 (diagnostics: add test case for trait bounds diagnostic)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit to rust-lang-ci/rust that referenced this pull request Sep 8, 2023
…llaumeGomez

Rollup of 8 pull requests

Successful merges:

 - rust-lang#115345 (MCP661: Move wasm32-wasi-preview1-threads target to Tier 2)
 - rust-lang#115604 (rustdoc: Render private fields in tuple struct as `/* private fields */`)
 - rust-lang#115624 (Print the path of a return-position impl trait in trait when `return_type_notation` is enabled)
 - rust-lang#115629 (Don't suggest dereferencing to unsized type)
 - rust-lang#115633 (Lint node for `PRIVATE_BOUNDS`/`PRIVATE_INTERFACES` is the item which names the private type)
 - rust-lang#115634 (Use `newtype_index` for `IntVid` and `FloatVid`.)
 - rust-lang#115638 (`-Cllvm-args` usability improvement)
 - rust-lang#115649 (diagnostics: add test case for trait bounds diagnostic)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit to rust-lang-ci/rust that referenced this pull request Sep 8, 2023
…llaumeGomez

Rollup of 6 pull requests

Successful merges:

 - rust-lang#104299 (Clarify stability guarantee for lifetimes in enum discriminants)
 - rust-lang#115088 (Fix Step Skipping Caused by Using the `--exclude` Option)
 - rust-lang#115201 (rustdoc: list matching impls on type aliases)
 - rust-lang#115633 (Lint node for `PRIVATE_BOUNDS`/`PRIVATE_INTERFACES` is the item which names the private type)
 - rust-lang#115638 (`-Cllvm-args` usability improvement)
 - rust-lang#115643 (fix: return early when has tainted in mir-lint)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 575c363 into rust-lang:master Sep 8, 2023
11 checks passed
@rustbot rustbot added this to the 1.74.0 milestone Sep 8, 2023
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Sep 8, 2023
Rollup merge of rust-lang#115638 - ldm0:ldm/llvm-args-fix, r=nikic

`-Cllvm-args` usability improvement

fixes: rust-lang#26338
fixes: rust-lang#115564

Two problems were found during playing with `-Cllvm-args`

1. When `llvm.link-shared` is set to `false` in `config.toml`, output of `rustc -C llvm-args='--help-list-hidden'` doesn't contain `--emit-dwarf-unwind` and `--emulated-tls`. When it is set to `true`, `rustc -C llvm-args='--help-list-hidden'` emits `--emit-dwarf-unwind`, but `--emulated-tls` is still missing.
2. Setting `-Cllvm-args=--emit-dwarf-unwind=always` doesn't take any effect, but `-Cllvm-args=-machine-outliner-reruns=3` does work.

### 1

Adding `RegisterCodeGenFlags` to register codegen flags fixed the first problem. `rustc -C llvm-args='--help-list-hidden'` emits full codegen flags including `--emit-dwarf-unwind` and `--emulated-tls`.

### 2

Constructing `TargetOptions` from `InitTargetOptionsFromCodeGenFlags` in `LLVMRustCreateTargetMachine` fixed the second problem. The `LLVMRustSetLLVMOptions` calls `ParseCommandLineOptions` which parses given `llvm-args`. For options like `machine-outliner-reruns`, it just works, since the codegen logic directly consumes the parsing result:

[machine-outliner-reruns register](https://github.com/rust-lang/llvm-project/blob/0537f6354cffe546cbf47f6dc9c7f82e49e86cfb/llvm/lib/CodeGen/MachineOutliner.cpp#L114)
[machine-outliner-reruns consumption](https://github.com/rust-lang/llvm-project/blob/0537f6354cffe546cbf47f6dc9c7f82e49e86cfb/llvm/lib/CodeGen/MachineOutliner.cpp#L1138)

But for flags defined in `TargetOptions` and `MCTargetOptions` to take effect, constructing them with `InitTargetOptionsFromCodeGenFlags` is essential, or the parsing result is just not consumed. Similar patterns can be observed in [lli](https://github.com/rust-lang/llvm-project/blob/0537f6354cffe546cbf47f6dc9c7f82e49e86cfb/llvm/tools/llc/llc.cpp#L494), [llc](https://github.com/rust-lang/llvm-project/blob/0537f6354cffe546cbf47f6dc9c7f82e49e86cfb/llvm/tools/lli/lli.cpp#L517), etc.
@ldm0 ldm0 deleted the ldm/llvm-args-fix branch September 9, 2023 10:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
5 participants