Skip to content

[WIP] Precise range metadata for enums with disjoint ranges #133836

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 1 commit into from
Closed

[WIP] Precise range metadata for enums with disjoint ranges #133836

wants to merge 1 commit into from

Conversation

clubby789
Copy link
Contributor

Fixes #133822

Very WIP

r? @ghost

@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 Dec 4, 2024
@rust-log-analyzer
Copy link
Collaborator

The job x86_64-gnu-llvm-18 failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot) 
#17 exporting to docker image format
#17 sending tarball 28.2s done
#17 DONE 33.7s
##[endgroup]
Setting extra environment values for docker:  --env ENABLE_GCC_CODEGEN=1 --env GCC_EXEC_PREFIX=/usr/lib/gcc/
[CI_JOB_NAME=x86_64-gnu-llvm-18]
debug: `DISABLE_CI_RUSTC_IF_INCOMPATIBLE` configured.
---
sccache: Starting the server...
##[group]Configure the build
configure: processing command line
configure: 
configure: build.configure-args := ['--build=x86_64-unknown-linux-gnu', '--llvm-root=/usr/lib/llvm-18', '--enable-llvm-link-shared', '--set', 'rust.randomize-layout=true', '--set', 'rust.thin-lto-import-instr-limit=10', '--enable-verbose-configure', '--enable-sccache', '--disable-manage-submodules', '--enable-locked-deps', '--enable-cargo-native-static', '--set', 'rust.codegen-units-std=1', '--set', 'dist.compression-profile=balanced', '--dist-compression-formats=xz', '--set', 'rust.lld=false', '--disable-dist-src', '--release-channel=nightly', '--enable-debug-assertions', '--enable-overflow-checks', '--enable-llvm-assertions', '--set', 'rust.verify-llvm-ir', '--set', 'rust.codegen-backends=llvm,cranelift,gcc', '--set', 'llvm.static-libstdcpp', '--enable-new-symbol-mangling']
configure: target.x86_64-unknown-linux-gnu.llvm-config := /usr/lib/llvm-18/bin/llvm-config
configure: llvm.link-shared     := True
configure: rust.randomize-layout := True
configure: rust.thin-lto-import-instr-limit := 10
---
   Compiling std v0.0.0 (/checkout/library/std)
   Compiling cc v1.2.0
   Compiling compiler_builtins v0.1.138
   Compiling rustc-std-workspace-core v1.99.0 (/checkout/library/rustc-std-workspace-core)
Intervals are not in order
!5 = !{i8 0, i8 2, i8 -1, i8 0}
Intervals are not in order
!5 = !{i8 0, i8 2, i8 -1, i8 0}
Intervals are not in order
!5 = !{i8 0, i8 2, i8 -1, i8 0}
Intervals are not in order
!5 = !{i8 0, i8 2, i8 -1, i8 0}
Intervals are not in order
!5 = !{i8 0, i8 2, i8 -1, i8 0}
Intervals are not in order
!5 = !{i8 0, i8 2, i8 -1, i8 0}
Intervals are not in order
!5 = !{i8 0, i8 2, i8 -1, i8 0}
Intervals are not in order
!5 = !{i8 0, i8 2, i8 -1, i8 0}
Intervals are not in order
!5 = !{i8 0, i8 2, i8 -1, i8 0}
Intervals are not in order
!5 = !{i8 0, i8 2, i8 -1, i8 0}
Intervals are not in order
!5 = !{i8 0, i8 2, i8 -1, i8 0}
Intervals are not in order
!5 = !{i8 0, i8 2, i8 -1, i8 0}
Intervals are not in order
!5 = !{i8 0, i8 2, i8 -1, i8 0}
rustc-LLVM ERROR: Broken module found, compilation aborted!

Caused by:
Caused by:
  process didn't exit successfully: `/checkout/obj/build/bootstrap/debug/rustc /checkout/obj/build/bootstrap/debug/rustc --crate-name core --edition=2021 library/core/src/lib.rs --error-format=json --json=diagnostic-rendered-ansi,artifacts,future-incompat --crate-type lib --emit=dep-info,metadata,link -C opt-level=3 -C embed-bitcode=no -C codegen-units=1 --warn=unexpected_cfgs --check-cfg 'cfg(bootstrap)' --check-cfg 'cfg(no_fp_fmt_parse)' --check-cfg 'cfg(stdarch_intel_sde)' --check-cfg 'cfg(feature, values(any()))' -C debug-assertions=on --check-cfg 'cfg(docsrs)' --check-cfg 'cfg(feature, values("debug_refcell", "optimize_for_size", "panic_immediate_abort"))' -C metadata=c9690195bf6f5d1b -C extra-filename=-c9690195bf6f5d1b --out-dir /checkout/obj/build/x86_64-unknown-linux-gnu/stage1-std/x86_64-unknown-linux-gnu/release/deps --target x86_64-unknown-linux-gnu -L dependency=/checkout/obj/build/x86_64-unknown-linux-gnu/stage1-std/x86_64-unknown-linux-gnu/release/deps -L dependency=/checkout/obj/build/x86_64-unknown-linux-gnu/stage1-std/release/deps -Csymbol-mangling-version=v0 -Zrandomize-layout '--check-cfg=cfg(feature,values(any()))' -Zunstable-options '--check-cfg=cfg(bootstrap)' -Zmacro-backtrace -Csplit-debuginfo=off -Cprefer-dynamic -Cllvm-args=-import-instr-limit=10 --cfg=randomized_layouts -Zinline-mir -Zinline-mir-preserve-debug -Clink-args=-Wl,-z,origin '-Clink-args=-Wl,-rpath,$ORIGIN/../lib' -Cembed-bitcode=yes -Cforce-frame-pointers=yes '-Zcrate-attr=doc(html_root_url="https://doc.rust-lang.org/nightly/")' -Z binary-dep-depinfo` (exit status: 101)
Build completed unsuccessfully in 0:05:28
  local time: Wed Dec  4 02:28:02 UTC 2024
  network time: Wed, 04 Dec 2024 02:28:02 GMT
##[error]Process completed with exit code 1.

scottmcm
scottmcm reviewed Dec 4, 2024
View reviewed changes
compiler/rustc_codegen_llvm/src/builder.rs
bx.set_metadata(load, llvm::MD_range, md);
}
}
} else if !scalar.is_always_valid(bx) {
bx.range_metadata(load, scalar.valid_range(bx));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To me, if you're going to do this, I would expect it to be on the Layout somehow, like valid_range is.

After all, if this is worth doing then it's probably worth doing for char as well, and I wouldn't want to duplicate everything about ranges in multiple places.

(But check with the LLVM folks to see whether this is even useful in the first place, since I don't know if multiple range metadata are even used.)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have never seen it being used anywhere, not even in this simplest example: https://alive2.llvm.org/ce/z/JEe2Q3.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds like not worth pursuing for now; thanks!

@clubby789 clubby789 closed this Dec 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dianqk dianqk dianqk left review comments

@scottmcm scottmcm scottmcm left review comments

Assignees
No one assigned
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
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

enums with disjoint ranges should emit more precise llvm.range metadata
5 participants
@clubby789 @rust-log-analyzer @dianqk @scottmcm @rustbot