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

Allow constants using an Abi::Vector layout to be passed to the backend #118171

Closed
wants to merge 1 commit into from

Conversation

GeorgeWort
Copy link
Contributor

This allows constant vectors using a repr(simd) type to be propagated through to the backend. This fixes a few issues with LLVM simd intrinsics where a constant vector is expected but cannot be produced by rust at opt-level=0.

@rustbot
Copy link
Collaborator

rustbot commented Nov 22, 2023

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @b-naber (or someone else) soon.

Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (S-waiting-on-review and S-waiting-on-author) stays updated, invoking these commands when appropriate:

  • @rustbot author: the review is finished, PR author should check the comments and take action accordingly
  • @rustbot review: the author is ready for a review, this PR will be queued again in the reviewer's queue

@rustbot
Copy link
Collaborator

rustbot commented Nov 22, 2023

Some changes occurred to the CTFE / Miri engine

cc @rust-lang/miri

Some changes occurred in compiler/rustc_codegen_gcc

cc @antoyo

@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 Nov 22, 2023
@rust-log-analyzer

This comment has been minimized.

This allows constant vectors using a repr(simd) type to be propagated
through to the backend. This fixes a few issues with LLVM simd intrinsics
where a constant vector is expected but cannot be produced by rust at
opt-level=0.
@rust-log-analyzer
Copy link
Collaborator

The job mingw-check failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)
GITHUB_ACTION=__run_7
GITHUB_ACTIONS=true
GITHUB_ACTION_REF=
GITHUB_ACTION_REPOSITORY=
GITHUB_ACTOR=GeorgeWort
GITHUB_API_URL=https://api.github.com
GITHUB_BASE_REF=master
GITHUB_ENV=/home/runner/work/_temp/_runner_file_commands/set_env_8f9e498c-8b21-4b41-bd1c-b87b454913f6
GITHUB_EVENT_NAME=pull_request
---
GITHUB_SERVER_URL=https://github.com
GITHUB_SHA=3ba119b511935de08f96807f9817939ff272a412
GITHUB_STATE=/home/runner/work/_temp/_runner_file_commands/save_state_8f9e498c-8b21-4b41-bd1c-b87b454913f6
GITHUB_STEP_SUMMARY=/home/runner/work/_temp/_runner_file_commands/step_summary_8f9e498c-8b21-4b41-bd1c-b87b454913f6
GITHUB_TRIGGERING_ACTOR=GeorgeWort
GITHUB_WORKFLOW_REF=rust-lang/rust/.github/workflows/ci.yml@refs/pull/118171/merge
GITHUB_WORKFLOW_SHA=3ba119b511935de08f96807f9817939ff272a412
GITHUB_WORKSPACE=/home/runner/work/rust/rust
GOROOT_1_19_X64=/opt/hostedtoolcache/go/1.19.13/x64
---
    Checking gccjit v1.0.0 (https://github.com/antoyo/gccjit.rs#6e290f25)
    Checking object v0.30.4
    Checking tempfile v3.7.1
    Checking rustc_codegen_gcc v0.1.0 (/checkout/compiler/rustc_codegen_gcc)
error[E0599]: no method named `type_ix` found for mutable reference `&mut builder::Builder<'a, 'gcc, 'tcx>` in the current scope
    |
    |
323 |                     self.sext(cmp, self.type_ix(32))
    |
    = help: items from traits can only be used if the trait is in scope
help: the following trait is implemented but not in scope; perhaps add a `use` for it:
    |
    |
1   + use crate::rustc_codegen_ssa::traits::BaseTypeMethods;
help: there is a method with a similar name
    |
    |
323 |                     self.sext(cmp, self.type_i8p_ext(32))

For more information about this error, try `rustc --explain E0599`.
error: could not compile `rustc_codegen_gcc` (lib) due to previous error
Build completed unsuccessfully in 0:02:26

@RalfJung
Copy link
Member

RalfJung commented Nov 22, 2023

Thanks for the PR! However, conceptually what you are suggesting here is a huge change and has to be done extremely carefully. I don't think we have sufficient motivation for such a fundamental refactor of our immediate handling. This needs a compiler MCP for sure.

The discussion here is also relevant: you are looking for a way to give vectors SSA variable treatment similar to Scalar and ScalarPair.

If we do want to go with this, we definitely want a new variant in the interpreter's Immediate, these should align with the variants of the Abi type. I'm not sure I even understand what the current representation is, but Immediate::Scalar is for scalar values only and a vector is not a scalar value.

@RalfJung
Copy link
Member

RalfJung commented Nov 22, 2023

You say this fixes some issue. Where's that issue tracked, where can I find the details? I sure hope there's an easier way to resolve that. Generally whenever LLVM requires something to be a constant we want to represent it as a const generic in Rust.

If you haven't done so already, please file an issue explaining the problem and your investigation of it.

@GeorgeWort
Copy link
Contributor Author

Thanks for the feedback! That makes sense. There may be alternative options to fix this issue, I would be happy to go with any better alternative that you can think of.

I have created a bug report here.

@bors
Copy link
Contributor

bors commented Nov 30, 2023

☔ The latest upstream changes (presumably #118408) made this pull request unmergeable. Please resolve the merge conflicts.

@Noratrieb Noratrieb added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Dec 18, 2023
@workingjubilee
Copy link
Member

#120690

r? compiler

@rustbot rustbot assigned compiler-errors and unassigned b-naber Feb 6, 2024
@Dylan-DPC
Copy link
Member

@GeorgeWort any updates on this? thanks

@GeorgeWort
Copy link
Contributor Author

I should have closed this PR sorry, all continuing related work is linked to from the issue #118209.

@GeorgeWort GeorgeWort closed this Aug 1, 2024
@Dylan-DPC Dylan-DPC removed the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Aug 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants