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

-Calign-loops=1 support in rustc #128832

Open
ojeda opened this issue Aug 8, 2024 · 5 comments
Open

-Calign-loops=1 support in rustc #128832

ojeda opened this issue Aug 8, 2024 · 5 comments
Labels
A-CLI Area: Command-line interface (CLI) to the compiler A-codegen Area: Code generation A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. A-rust-for-linux Relevant for the Rust-for-Linux project C-feature-request Category: A feature request, i.e: not implemented / a PR. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@ojeda
Copy link
Contributor

ojeda commented Aug 8, 2024

i.e. the equivalent of -falign-loops=1 (GCC, Clang).

@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Aug 8, 2024
@ojeda ojeda changed the title -Calign-jumps=1 support in rustc -Calign-loops=1 support in rustc Aug 8, 2024
@ojeda
Copy link
Contributor Author

ojeda commented Aug 8, 2024

@rustbot label I-rust-for-linux

@rustbot rustbot added the A-rust-for-linux Relevant for the Rust-for-Linux project label Aug 8, 2024
@jieyouxu jieyouxu added T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. C-feature-request Category: A feature request, i.e: not implemented / a PR. and removed needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. labels Aug 8, 2024
@workingjubilee workingjubilee added A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. A-codegen Area: Code generation labels Aug 8, 2024
@workingjubilee workingjubilee added the A-CLI Area: Command-line interface (CLI) to the compiler label Oct 22, 2024
@folkertdev
Copy link
Contributor

This behavior can already be accessed with e.g. -Cllvm-args="--align-loops=32", see https://godbolt.org/z/eEc1oP7MY. When you change the alignment, the p2align directives on line 44 and 72 change their alignment to match.

So I think rust does not need custom support for this flag?

@ojeda
Copy link
Contributor Author

ojeda commented Dec 8, 2024

Thanks, that is good, but what about other backends, e.g. GCC? This flag may not be commonly used, but should we leave it up to per-backend flags?

@folkertdev
Copy link
Contributor

well with this flag you'd always be passing a flag on to the backend: rustc itself does not know what loops are hot or cold (in this it is different from e.g. the function alignment, where we can explicitly set the alignment of each function).

By making it a rust flag, you'd have to guarantee consistent behavior between targets (I would think, anyway). Can we guarantee that gcc and clang do the same thing here (in the sense that the rust specification could describe their behavior)? And e.g. cranelift does not support loop alignment at all as far as I can tell.

Making this the responsibility of the backend, via some per-backend configuration flag seems like by far the simplest option. But if there really is a pressing need maybe something more is possible? I don't know, and don't have any authority here.

@ojeda
Copy link
Contributor Author

ojeda commented Dec 8, 2024

well with this flag you'd always be passing a flag on to the backend:

What I meant was that projects would need to use those per-backend flags themselves. That is, every project would need to repeat that logic on their side (for flags that are supported by e.g. both GCC and Clang). That may be fine, since projects needing this kind of flag can probably figure it out and there may not be many.

By making it a rust flag, you'd have to guarantee consistent behavior between targets (I would think, anyway).

I don't know what is the policy here -- do flags need to require consistent behavior? Could the documentation avoid specifying details where they may not be guaranteed? Or, could it state that it behaves like -falign-loops for the GCC and Clang backends? For Cranelift etc., it could perhaps be an error to apply it to backends for which it does not make sense, or it could be defined as not having an effect.

Making this the responsibility of the backend, via some per-backend configuration flag seems like by far the simplest option.

If those per-backend flags are stable, then that would be good enough for us, yeah. In that case, it may be a good idea to document these.

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-CLI Area: Command-line interface (CLI) to the compiler A-codegen Area: Code generation A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. A-rust-for-linux Relevant for the Rust-for-Linux project C-feature-request Category: A feature request, i.e: not implemented / a PR. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

5 participants