Skip to content

Commit 3701847

Browse files
authored
Rollup merge of rust-lang#139098 - scottmcm:assert-impossible-tags, r=WaffleLapkin
Tell LLVM about impossible niche tags I was trying to find a better way of emitting discriminant calculations, but sadly had no luck. So here's a fairly small PR with the bits that did seem worth bothering: 1. As the [`TagEncoding::Niche` docs](https://doc.rust-lang.org/nightly/nightly-rustc/rustc_abi/enum.TagEncoding.html#variant.Niche) describe, it's possible to end up with a dead value in the input that's not already communicated via the range parameter attribute nor the range load metadata attribute. So this adds an `llvm.assume` in non-debug mode to tell LLVM about that. (That way it can tell that the sides of the `select` have disjoint possible values.) 2. I'd written a bunch more tests, or at least made them parameterized, in the process of trying things out, so this checks in those tests to hopefully help future people not trip on the same weird edge cases, like when the tag type is `i8` but yet there's still a variant index and discriminant of `258` which doesn't fit in that tag type because the enum is really weird.
2 parents 8a31cc2 + 502f7f9 commit 3701847

File tree

3 files changed

+487
-20
lines changed

3 files changed

+487
-20
lines changed

compiler/rustc_codegen_ssa/src/mir/operand.rs

+31-1
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ use rustc_middle::mir::{self, ConstValue};
99
use rustc_middle::ty::Ty;
1010
use rustc_middle::ty::layout::{LayoutOf, TyAndLayout};
1111
use rustc_middle::{bug, span_bug};
12+
use rustc_session::config::OptLevel;
1213
use tracing::{debug, instrument};
1314

1415
use super::place::{PlaceRef, PlaceValue};
@@ -496,6 +497,18 @@ impl<'a, 'tcx, V: CodegenObject> OperandRef<'tcx, V> {
496497
_ => (tag_imm, bx.cx().immediate_backend_type(tag_op.layout)),
497498
};
498499

500+
// Layout ensures that we only get here for cases where the discriminant
501+
// value and the variant index match, since that's all `Niche` can encode.
502+
// But for emphasis and debugging, let's double-check one anyway.
503+
debug_assert_eq!(
504+
self.layout
505+
.ty
506+
.discriminant_for_variant(bx.tcx(), untagged_variant)
507+
.unwrap()
508+
.val,
509+
u128::from(untagged_variant.as_u32()),
510+
);
511+
499512
let relative_max = niche_variants.end().as_u32() - niche_variants.start().as_u32();
500513

501514
// We have a subrange `niche_start..=niche_end` inside `range`.
@@ -537,6 +550,21 @@ impl<'a, 'tcx, V: CodegenObject> OperandRef<'tcx, V> {
537550
relative_discr,
538551
bx.cx().const_uint(tag_llty, relative_max as u64),
539552
);
553+
554+
// Thanks to parameter attributes and load metadata, LLVM already knows
555+
// the general valid range of the tag. It's possible, though, for there
556+
// to be an impossible value *in the middle*, which those ranges don't
557+
// communicate, so it's worth an `assume` to let the optimizer know.
558+
if niche_variants.contains(&untagged_variant)
559+
&& bx.cx().sess().opts.optimize != OptLevel::No
560+
{
561+
let impossible =
562+
u64::from(untagged_variant.as_u32() - niche_variants.start().as_u32());
563+
let impossible = bx.cx().const_uint(tag_llty, impossible);
564+
let ne = bx.icmp(IntPredicate::IntNE, relative_discr, impossible);
565+
bx.assume(ne);
566+
}
567+
540568
(is_niche, cast_tag, niche_variants.start().as_u32() as u128)
541569
};
542570

@@ -553,7 +581,9 @@ impl<'a, 'tcx, V: CodegenObject> OperandRef<'tcx, V> {
553581
);
554582

555583
// In principle we could insert assumes on the possible range of `discr`, but
556-
// currently in LLVM this seems to be a pessimization.
584+
// currently in LLVM this isn't worth it because the original `tag` will
585+
// have either a `range` parameter attribute or `!range` metadata,
586+
// or come from a `transmute` that already `assume`d it.
557587

558588
discr
559589
}

0 commit comments

Comments
 (0)