Skip to content
This repository was archived by the owner on Oct 31, 2025. It is now read-only.

Fix some ICEs with bools #826

Merged
khyperia merged 2 commits into from
Dec 15, 2021
Merged

Fix some ICEs with bools #826

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
530f620
Fix some ICEs with bools
khyperia Dec 15, 2021
0ac4408
:sweat_smile:
khyperia Dec 15, 2021
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
16 changes: 6 additions & 10 deletions crates/rustc_codegen_spirv/src/builder/builder_methods.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -927,20 +927,16 @@ impl<'a, 'tcx> BuilderMethods<'a, 'tcx> for Builder<'a, 'tcx> {
let b_offset = a.value.size(self).align_to(b.value.align(self).abi);

let pair_ty = place.layout.spirv_type(self.span(), self);
let mut load = |i, scalar: &Scalar, align| {
let mut load = |i, _: &Scalar, align| {
let llptr = self.struct_gep(pair_ty, place.llval, i as u64);
let load = self.load(
// WARN! This should go through to_immediate, but because we only have a Scalar,
// not a Ty, we can't call to_immediate here (even though we implement it as a
// no-op), so "inline" the no-op here and do nothing.
Copy link
Contributor

@eddyb eddyb Dec 15, 2021

Choose a reason for hiding this comment

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

Isn't that why to_immediate_scalar exists, and to_immediate merely proxies to it? rustc_codegen_ssa calls to_immediate_scalar in a few places where to_immediate isn't available (e.g. for each half of a ScalarPair).

Copy link
Contributor Author

@khyperia khyperia Dec 15, 2021

Choose a reason for hiding this comment

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

ahah, yes, I totally remembered to_immediate_scalar existed, mhmm, definitely. I'm going to go with the excuse that "I don't think it existed when I originally wrote this code, and I just copied the comment/behavior forward with this new change" and hope that's right ✨

self.load(
self.scalar_pair_element_backend_type(place.layout, i, false),
llptr,
align,
);
// WARN! This does not go through to_immediate due to only having a Scalar, not a Ty, but it still does
// whatever to_immediate does!
if scalar.is_bool() {
self.trunc(load, SpirvType::Bool.def(self.span(), self))
} else {
load
}
)
};

OperandValue::Pair(
Expand Down
12 changes: 1 addition & 11 deletions crates/rustc_codegen_spirv/src/codegen_cx/declare.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -263,7 +263,6 @@ impl<'tcx> StaticMethods for CodegenCx<'tcx> {

fn codegen_static(&self, def_id: DefId, _is_mutable: bool) {
let g = self.get_static(def_id);
let span = self.tcx.def_span(def_id);

let alloc = match self.tcx.eval_static_initializer(def_id) {
Ok(alloc) => alloc,
Expand All @@ -277,16 +276,7 @@ impl<'tcx> StaticMethods for CodegenCx<'tcx> {
other.debug(g.ty, self)
)),
};
let mut v = self.create_const_alloc(alloc, value_ty);

if self.lookup_type(v.ty) == SpirvType::Bool {
let val_int = match self.builder.lookup_const(v).unwrap() {
SpirvConst::Bool(val) => val as u8,
_ => bug!(),
};
v = self.constant_u8(span, val_int);
}

let v = self.create_const_alloc(alloc, value_ty);
assert_ty_eq!(self, value_ty, v.ty);
self.builder
.set_global_initializer(g.def_cx(self), v.def_cx(self));
Expand Down