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

Use LLVM patch to correct alignments for i128 and u128 #116641

Closed
wants to merge 3 commits into from

Conversation

tgross35
Copy link
Contributor

@tgross35 tgross35 commented Oct 11, 2023

This applies https://reviews.llvm.org/D86310 and https://reviews.llvm.org/D158169, which fix the alignment and ABI issues for 128-bit integers. I ran an ABI check on this and we are no compatible with GCC and the current main clang.

Zulip discussion: https://rust-lang.zulipchat.com/#narrow/stream/131828-t-compiler/topic/.2354341.20-.20alignment.20of.20i128.20for.20FFI

This will be a draft since I don't think we are going to backport

r? compiler
@rustbot label +T-compiler

Todo still: pull in the rest of the changes from #113880

@tgross35 tgross35 marked this pull request as draft October 11, 2023 18:21
@rustbot
Copy link
Collaborator

rustbot commented Oct 11, 2023

⚠️ Warning ⚠️

  • These commits modify submodules.

@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 Oct 11, 2023
@rustbot
Copy link
Collaborator

rustbot commented Oct 11, 2023

These commits modify compiler targets.
(See the Target Tier Policy.)

@rust-log-analyzer
Copy link
Collaborator

The job x86_64-gnu-llvm-15 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=tgross35
GITHUB_API_URL=https://api.github.com
GITHUB_BASE_REF=master
GITHUB_ENV=/home/runner/work/_temp/_runner_file_commands/set_env_0f20dd68-449d-43f5-a1c4-4035f3466203
GITHUB_EVENT_NAME=pull_request
GITHUB_EVENT_NAME=pull_request
GITHUB_EVENT_PATH=/home/runner/work/_temp/_github_workflow/event.json
GITHUB_GRAPHQL_URL=https://api.github.com/graphql
GITHUB_HEAD_REF=i128-fix
GITHUB_JOB=pr
GITHUB_PATH=/home/runner/work/_temp/_runner_file_commands/add_path_0f20dd68-449d-43f5-a1c4-4035f3466203
GITHUB_REF=refs/pull/116641/merge
GITHUB_REF_NAME=116641/merge
GITHUB_REF_PROTECTED=false
---
GITHUB_SERVER_URL=https://github.com
GITHUB_SHA=a97ea427a5a192a7cd9ef7e2ab0a3feb6c919f06
GITHUB_STATE=/home/runner/work/_temp/_runner_file_commands/save_state_0f20dd68-449d-43f5-a1c4-4035f3466203
GITHUB_STEP_SUMMARY=/home/runner/work/_temp/_runner_file_commands/step_summary_0f20dd68-449d-43f5-a1c4-4035f3466203
GITHUB_TRIGGERING_ACTOR=tgross35
GITHUB_WORKFLOW_REF=rust-lang/rust/.github/workflows/ci.yml@refs/pull/116641/merge
GITHUB_WORKFLOW_SHA=a97ea427a5a192a7cd9ef7e2ab0a3feb6c919f06
GITHUB_WORKSPACE=/home/runner/work/rust/rust
GOROOT_1_19_X64=/opt/hostedtoolcache/go/1.19.13/x64
---
Submodule 'src/doc/nomicon' (https://github.com/rust-lang/nomicon.git) registered for path 'src/doc/nomicon'
Submodule 'src/doc/reference' (https://github.com/rust-lang/reference.git) registered for path 'src/doc/reference'
Submodule 'src/doc/rust-by-example' (https://github.com/rust-lang/rust-by-example.git) registered for path 'src/doc/rust-by-example'
Submodule 'src/doc/rustc-dev-guide' (https://github.com/rust-lang/rustc-dev-guide.git) registered for path 'src/doc/rustc-dev-guide'
Submodule 'src/llvm-project' (https://github.com/tgross35/llvm-project.git) registered for path 'src/llvm-project'
Cloning into '/home/runner/work/rust/rust/library/backtrace'...
Cloning into '/home/runner/work/rust/rust/src/doc/edition-guide'...
Cloning into '/home/runner/work/rust/rust/src/doc/nomicon'...
Cloning into '/home/runner/work/rust/rust/library/stdarch'...
---
Submodule path 'src/doc/rust-by-example': checked out '8eb3a01ab74c567b7174784892fb807f2c632d6b'
From https://github.com/rust-lang/rustc-dev-guide
 * branch            b98af7d661e4744baab81fb8dc7a049e44a4a998 -> FETCH_HEAD
Submodule path 'src/doc/rustc-dev-guide': checked out 'b98af7d661e4744baab81fb8dc7a049e44a4a998'
From https://github.com/tgross35/llvm-project
Submodule path 'src/llvm-project': checked out '8cd0d3b87f27c59df147883d83ebd84015c33de8'
From https://github.com/rust-lang/cargo
 * branch            6fa6fdc7606cfa664f9bee2fb33ee2ed904f4e88 -> FETCH_HEAD
Submodule path 'src/tools/cargo': checked out '6fa6fdc7606cfa664f9bee2fb33ee2ed904f4e88'
---
   Compiling rustc_fluent_macro v0.1.0 (/checkout/compiler/rustc_fluent_macro)
error[E0308]: mismatched types
    --> /checkout/compiler/rustc_index/src/lib.rs:47:32
     |
45   | macro_rules! static_assert_size {
     | ------------------------------- in this expansion of `static_assert_size!`
46   |     ($ty:ty, $size:expr) => {
47   |         const _: [(); $size] = [(); ::std::mem::size_of::<$ty>()];
     |                                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected an array with a fixed size of 24 elements, found one with 32 elements
    ::: compiler/rustc_ast/src/ast.rs:3188:34
     |
     |
3188 |     static_assert_size!(LitKind, 24);
     |     |                            |
     |     |                            |
     |     |                            help: consider specifying the actual array length: `32`

error[E0308]: mismatched types
    --> /checkout/compiler/rustc_index/src/lib.rs:47:32
     |
     |
45   | macro_rules! static_assert_size {
     | ------------------------------- in this expansion of `static_assert_size!`
46   |     ($ty:ty, $size:expr) => {
47   |         const _: [(); $size] = [(); ::std::mem::size_of::<$ty>()];
     |                                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected an array with a fixed size of 40 elements, found one with 48 elements
    ::: compiler/rustc_ast/src/ast.rs:3190:38
     |
     |
3190 |     static_assert_size!(MetaItemLit, 40);
     |     |                                |
     |     |                                |
     |     |                                help: consider specifying the actual array length: `48`

   Compiling rustc_error_messages v0.0.0 (/checkout/compiler/rustc_error_messages)
For more information about this error, try `rustc --explain E0308`.
error: could not compile `rustc_ast` (lib) due to 2 previous errors

krasimirgg added a commit to krasimirgg/rust that referenced this pull request Oct 12, 2023
This is a clone of rust-lang#116641
with a few static asserts disabled, specifically to try and bootstrap
rustc with LLVM at HEAD.

This is just for preliminary testing, will drop this in favor of
rust-lang#116641 once ready.
@krasimirgg
Copy link
Contributor

krasimirgg commented Oct 12, 2023

It seems we may need to also update i686 specs: when locally building this with LLVM @ HEAD and the static_assert_size-s commented out following #113880, I'm getting some i686 test failures, e.g., tests/codegen/abi-efiapi.rs#i686:

compiler/rustc_codegen_llvm/src/context.rs:190:13: data-layout for target `i686-unknown-linux-musl`, `e-m:e-p:32:32-p270:32:32-p271:32:32-p272:64:64-f64:32:64-f80:32-n8:16:32-S128`, differs from LLVM target's `i686-unknown-linux-musl` default layout, `e-m:e-p:32:32-p270:32:32-p271:32:32-p272:64:64-i128:128-f64:32:64-f80:32-n8:16:32-S128`

Also these which seem to use i686-pc-windows-msvc:

test [codegen] tests/codegen/unwind-abis/fastcall-unwind-abi.rs ... FAILED
test [codegen] tests/codegen/unwind-abis/stdcall-unwind-abi.rs ... FAILED
test [codegen] tests/codegen/unwind-abis/thiscall-unwind-abi.rs ... FAILED
test [codegen] tests/codegen/unwind-abis/vectorcall-unwind-abi.rs ... FAILED

@oli-obk
Copy link
Contributor

oli-obk commented Oct 12, 2023

r? llvm

@rustbot
Copy link
Collaborator

rustbot commented Oct 12, 2023

Failed to set assignee to llvm: cannot assign: response: {"message":"Not Found","documentation_url":"https://docs.github.com/rest/issues/assignees#add-assignees-to-an-issue"}

Note: Only org members with at least the repository "read" role, users with write permissions, or people who have commented on the PR may be assigned.

@oli-obk
Copy link
Contributor

oli-obk commented Oct 12, 2023

r? @nikic

@rustbot rustbot assigned nikic and unassigned oli-obk Oct 12, 2023
@nikic
Copy link
Contributor

nikic commented Oct 12, 2023

@krasimirgg Yes, the i686 data layouts changed as well.

@tgross35
Copy link
Contributor Author

I think this may be superseded by #116672?

@tgross35
Copy link
Contributor Author

#116672 will handle this now, closing

@tgross35 tgross35 closed this Oct 13, 2023
@tgross35 tgross35 deleted the i128-fix branch March 5, 2024 00:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants