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

Closed
wants to merge 1 commit into from
Closed

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

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
42 changes: 41 additions & 1 deletion compiler/rustc_codegen_llvm/src/builder.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -505,7 +505,47 @@ impl<'a, 'll, 'tcx> BuilderMethods<'a, 'tcx> for Builder<'a, 'll, 'tcx> {

match scalar.primitive() {
abi::Primitive::Int(..) => {
if !scalar.is_always_valid(bx) {
if let Some(adt) = layout.ty.ty_adt_def()
&& adt.is_enum()
{
let mut discr_vals: Vec<_> =
adt.discriminants(bx.tcx).map(|(_, d)| d.val).collect();
discr_vals.sort();
let mut ranges =
vec![WrappingRange { start: discr_vals[0], end: discr_vals[0] }];
for &discr in &discr_vals[1..] {
let last = ranges.last_mut().unwrap();
if discr == last.end + 1 {
last.end = discr;
} else if !last.contains(discr) {
ranges.push(WrappingRange { start: discr, end: discr });
}
}

if bx.cx.sess().opts.optimize != OptLevel::No {
unsafe {
let llty = bx.cx.val_ty(load);
let md =
ranges
.iter()
.flat_map(|range| {
[
llvm::LLVMValueAsMetadata(
bx.cx.const_uint_big(llty, range.start),
),
llvm::LLVMValueAsMetadata(bx.cx.const_uint_big(
llty,
range.end.wrapping_add(1),
)),
]
})
.collect::<Vec<_>>();
let md =
llvm::LLVMMDNodeInContext2(bx.cx.llcx, md.as_ptr(), md.len());
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!

}
}
Expand Down
Loading