Skip to content

Commit

Permalink
cranelift/x64: Rely on zero-extended constants
Browse files Browse the repository at this point in the history
Since #6850, we've been able to rely on `iconst` instructions having
their immediate operands' high bits zeroed before lowering.

So a couple of places in `x64/lower.rs` can be expressed more simply now
as a result.

Out of an abundance of caution, I added a debug-assertion when constants
are looked up during lowering, to check that earlier phases really did
ensure the high bits are zero.

I also got rid of an `expect` where a simple pattern-match will do.
  • Loading branch information
jameysharp committed Apr 4, 2024
1 parent 646169e commit ffbc410
Show file tree
Hide file tree
Showing 2 changed files with 30 additions and 35 deletions.
55 changes: 21 additions & 34 deletions cranelift/codegen/src/isa/x64/lower.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
pub(super) mod isle;

use crate::ir::pcc::{FactContext, PccResult};
use crate::ir::{types, ExternalName, Inst as IRInst, LibCall, Opcode, Type};
use crate::ir::{types, ExternalName, Inst as IRInst, InstructionData, LibCall, Opcode, Type};
use crate::isa::x64::abi::*;
use crate::isa::x64::inst::args::*;
use crate::isa::x64::inst::*;
Expand Down Expand Up @@ -61,11 +61,10 @@ fn put_input_in_regs(ctx: &mut Lower<Inst>, spec: InsnInput) -> ValueRegs<Reg> {

if let Some(c) = input.constant {
// Generate constants fresh at each use to minimize long-range register pressure.
let from_bits = ty_bits(ty);
let (size, c) = if from_bits < 64 {
(OperandSize::Size32, c & ((1u64 << from_bits) - 1))
let size = if ty_bits(ty) < 64 {
OperandSize::Size32
} else {
(OperandSize::Size64, c)
OperandSize::Size64
};
assert!(is_int_or_ref_ty(ty)); // Only used for addresses.
let cst_copy = ctx.alloc_tmp(ty);
Expand Down Expand Up @@ -124,16 +123,18 @@ fn is_mergeable_load(
// Just testing the opcode is enough, because the width will always match if
// the type does (and the type should match if the CLIF is properly
// constructed).
if insn_data.opcode() == Opcode::Load {
let offset = insn_data
.load_store_offset()
.expect("load should have offset");
if let &InstructionData::Load {
opcode: Opcode::Load,
offset,
..
} = insn_data
{
Some((
InsnInput {
insn: src_insn,
input: 0,
},
offset,
offset.into(),
))
} else {
None
Expand Down Expand Up @@ -261,35 +262,21 @@ fn lower_to_amode(ctx: &mut Lower<Inst>, spec: InsnInput, offset: i32) -> Amode
shift_amt,
)
} else {
for i in 0..=1 {
for input in 0..=1 {
// Try to pierce through uextend.
if let Some(uextend) = matches_input(
ctx,
InsnInput {
insn: add,
input: i,
},
Opcode::Uextend,
) {
if let Some(cst) = ctx.get_input_as_source_or_const(uextend, 0).constant {
// Zero the upper bits.
let input_size = ctx.input_ty(uextend, 0).bits() as u64;
let shift: u64 = 64 - input_size;
let uext_cst: u64 = (cst << shift) >> shift;

let final_offset = (offset as i64).wrapping_add(uext_cst as i64);
if let Ok(final_offset) = i32::try_from(final_offset) {
let base = put_input_in_reg(ctx, add_inputs[1 - i]);
return Amode::imm_reg(final_offset, base).with_flags(flags);
}
}
}
let (inst, inst_input) = if let Some(uextend) =
matches_input(ctx, InsnInput { insn: add, input }, Opcode::Uextend)
{
(uextend, 0)
} else {
(add, input)
};

// If it's a constant, add it directly!
if let Some(cst) = ctx.get_input_as_source_or_const(add, i).constant {
if let Some(cst) = ctx.get_input_as_source_or_const(inst, inst_input).constant {
let final_offset = (offset as i64).wrapping_add(cst as i64);
if let Ok(final_offset) = i32::try_from(final_offset) {
let base = put_input_in_reg(ctx, add_inputs[1 - i]);
let base = put_input_in_reg(ctx, add_inputs[1 - input]);
return Amode::imm_reg(final_offset, base).with_flags(flags);
}
}
Expand Down
10 changes: 9 additions & 1 deletion cranelift/codegen/src/machinst/lower.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1218,7 +1218,15 @@ impl<'func, I: VCodeInst> Lower<'func, I> {
/// Get the value of a constant instruction (`iconst`, etc.) as a 64-bit
/// value, if possible.
pub fn get_constant(&self, ir_inst: Inst) -> Option<u64> {
self.inst_constants.get(&ir_inst).cloned()
self.inst_constants.get(&ir_inst).copied().inspect(|&c| {
// The upper bits must be zero, enforced during legalization and by
// the CLIF verifier.
debug_assert_eq!(c, {
let input_size = self.output_ty(ir_inst, 0).bits() as u64;
let shift = 64 - input_size;
(c << shift) >> shift
});
})
}

/// Get the input as one of two options other than a direct register:
Expand Down

0 comments on commit ffbc410

Please sign in to comment.