Skip to content
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.

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

PCC: rework default-facts somewhat. #7262

Merged
merged 1 commit into from
Oct 17, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
7 changes: 2 additions & 5 deletions cranelift/codegen/src/ir/memtype.rs
Original file line number Diff line number Diff line change
Expand Up @@ -160,12 +160,9 @@ pub struct MemoryTypeField {
}

impl MemoryTypeField {
/// Get the fact, if any, on a field. Fills in a default inferred
/// fact based on the type if no explicit fact is present.
/// Get the fact, if any, on a field.
pub fn fact(&self) -> Option<&Fact> {
self.fact
.as_ref()
.or_else(|| Fact::infer_from_type(self.ty))
self.fact.as_ref()
}
}

Expand Down
11 changes: 1 addition & 10 deletions cranelift/codegen/src/ir/pcc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -230,14 +230,6 @@ impl<'a> FactContext<'a> {
// Reflexivity.
(l, r) if l == r => true,

// Any value on the LHS subsumes a minimal (always-true)
// fact about the max value of a given bitwidth on the
// RHS: e.g., no matter the value, the bottom 8 bits will
// always be <= 255.
(_, Fact::ValueMax { bit_width, max }) if *max == max_value_for_width(*bit_width) => {
true
}

(
Fact::ValueMax {
bit_width: bw_lhs,
Expand Down Expand Up @@ -487,8 +479,7 @@ impl<'a> FactContext<'a> {
pub fn load<'b>(&'b self, fact: &Fact, access_ty: ir::Type) -> PccResult<Option<&'b Fact>> {
Ok(self
.struct_field(fact, access_ty)?
.and_then(|field| field.fact())
.or_else(|| Fact::infer_from_type(access_ty)))
.and_then(|field| field.fact()))
}

/// Check a store.
Expand Down
66 changes: 34 additions & 32 deletions cranelift/codegen/src/isa/aarch64/pcc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
use crate::ir::pcc::*;
use crate::ir::types::*;
use crate::ir::MemFlags;
use crate::ir::Type;
use crate::isa::aarch64::inst::args::{PairAMode, ShiftOp};
use crate::isa::aarch64::inst::ALUOp;
use crate::isa::aarch64::inst::Inst;
Expand All @@ -11,8 +12,11 @@ use crate::machinst::Reg;
use crate::machinst::VCode;
use crate::trace;

fn get_fact(vcode: &VCode<Inst>, reg: Reg) -> PccResult<&Fact> {
vcode.vreg_fact(reg.into()).ok_or(PccError::MissingFact)
fn get_fact_or_default(vcode: &VCode<Inst>, reg: Reg) -> PccResult<&Fact> {
vcode
.vreg_fact(reg.into())
.or_else(|| Fact::infer_from_type(vcode.vreg_type(reg.into())))
.ok_or(PccError::MissingFact)
}

fn has_fact(vcode: &VCode<Inst>, reg: Reg) -> bool {
Expand Down Expand Up @@ -130,21 +134,19 @@ pub(crate) fn check(ctx: &FactContext, vcode: &VCode<Inst>, inst: &Inst) -> PccR
rd,
rn,
rm,
} if has_fact(vcode, *rn) && has_fact(vcode, *rm) => {
check_output(&ctx, vcode, rd.to_reg(), || {
let rn = get_fact(vcode, *rn)?;
let rm = get_fact(vcode, *rm)?;
fail_if_missing(ctx.add(rn, rm, size.bits().into()))
})
}
} => check_output(&ctx, vcode, rd.to_reg(), || {
let rn = get_fact_or_default(vcode, *rn)?;
let rm = get_fact_or_default(vcode, *rm)?;
fail_if_missing(ctx.add(rn, rm, size.bits().into()))
}),
Inst::AluRRImm12 {
alu_op: ALUOp::Add,
size,
rd,
rn,
imm12,
} if has_fact(vcode, *rn) => check_output(&ctx, vcode, rd.to_reg(), || {
let rn = get_fact(vcode, *rn)?;
} => check_output(&ctx, vcode, rd.to_reg(), || {
let rn = get_fact_or_default(vcode, *rn)?;
let imm_fact = Fact::ValueMax {
bit_width: size.bits().into(),
max: imm12.value(),
Expand All @@ -160,8 +162,8 @@ pub(crate) fn check(ctx: &FactContext, vcode: &VCode<Inst>, inst: &Inst) -> PccR
shiftop,
} if shiftop.op() == ShiftOp::LSL && has_fact(vcode, *rn) && has_fact(vcode, *rm) => {
check_output(&ctx, vcode, rd.to_reg(), || {
let rn = get_fact(vcode, *rn)?;
let rm = get_fact(vcode, *rm)?;
let rn = get_fact_or_default(vcode, *rn)?;
let rm = get_fact_or_default(vcode, *rm)?;
let rm_shifted = fail_if_missing(ctx.shl(
&rm,
size.bits().into(),
Expand All @@ -179,8 +181,8 @@ pub(crate) fn check(ctx: &FactContext, vcode: &VCode<Inst>, inst: &Inst) -> PccR
extendop,
} if has_fact(vcode, *rn) && has_fact(vcode, *rm) => {
check_output(&ctx, vcode, rd.to_reg(), || {
let rn = get_fact(vcode, *rn)?;
let rm = get_fact(vcode, *rm)?;
let rn = get_fact_or_default(vcode, *rn)?;
let rm = get_fact_or_default(vcode, *rm)?;
let rm_extended = fail_if_missing(extend_fact(&ctx, rm, *extendop))?;
fail_if_missing(ctx.add(&rn, &rm_extended, size.bits().into()))
})
Expand All @@ -193,7 +195,7 @@ pub(crate) fn check(ctx: &FactContext, vcode: &VCode<Inst>, inst: &Inst) -> PccR
immshift,
} if has_fact(vcode, *rn) && has_fact(vcode, *rn) => {
check_output(&ctx, vcode, rd.to_reg(), || {
let rn = get_fact(vcode, *rn)?;
let rn = get_fact_or_default(vcode, *rn)?;
fail_if_missing(ctx.shl(&rn, size.bits().into(), immshift.value().into()))
})
}
Expand All @@ -204,7 +206,7 @@ pub(crate) fn check(ctx: &FactContext, vcode: &VCode<Inst>, inst: &Inst) -> PccR
from_bits,
to_bits,
} if has_fact(vcode, *rn) => check_output(&ctx, vcode, rd.to_reg(), || {
let rn = get_fact(vcode, *rn)?;
let rn = get_fact_or_default(vcode, *rn)?;
fail_if_missing(ctx.uextend(&rn, (*from_bits).into(), (*to_bits).into()))
}),
Inst::AluRRR { size, rd, .. }
Expand Down Expand Up @@ -249,7 +251,7 @@ fn check_load(
vcode: &VCode<Inst>,
ty: Type,
) -> PccResult<()> {
let result_fact = rd.map(|rd| get_fact(vcode, rd)).transpose()?;
let result_fact = rd.and_then(|rd| vcode.vreg_fact(rd.into()));
check_addr(
ctx,
flags,
Expand All @@ -268,7 +270,7 @@ fn check_store(
vcode: &VCode<Inst>,
ty: Type,
) -> PccResult<()> {
let stored_fact = rd.map(|rd| get_fact(vcode, rd)).transpose()?;
let stored_fact = rd.and_then(|rd| vcode.vreg_fact(rd.into()));
check_addr(
ctx,
flags,
Expand Down Expand Up @@ -312,14 +314,14 @@ fn check_addr<'a>(

match addr {
&AMode::RegReg { rn, rm } => {
let rn = get_fact(vcode, rn)?;
let rm = get_fact(vcode, rm)?;
let rn = get_fact_or_default(vcode, rn)?;
let rm = get_fact_or_default(vcode, rm)?;
let sum = fail_if_missing(ctx.add(&rn, &rm, 64))?;
check(&sum, ty)
}
&AMode::RegScaled { rn, rm, ty } => {
let rn = get_fact(vcode, rn)?;
let rm = get_fact(vcode, rm)?;
let rn = get_fact_or_default(vcode, rn)?;
let rm = get_fact_or_default(vcode, rm)?;
let rm_scaled = fail_if_missing(ctx.scale(&rm, 64, ty.bytes()))?;
let sum = fail_if_missing(ctx.add(&rn, &rm_scaled, 64))?;
check(&sum, ty)
Expand All @@ -330,16 +332,16 @@ fn check_addr<'a>(
ty,
extendop,
} => {
let rn = get_fact(vcode, rn)?;
let rm = get_fact(vcode, rm)?;
let rn = get_fact_or_default(vcode, rn)?;
let rm = get_fact_or_default(vcode, rm)?;
let rm_extended = fail_if_missing(extend_fact(ctx, rm, extendop))?;
let rm_scaled = fail_if_missing(ctx.scale(&rm_extended, 64, ty.bytes()))?;
let sum = fail_if_missing(ctx.add(&rn, &rm_scaled, 64))?;
check(&sum, ty)
}
&AMode::RegExtended { rn, rm, extendop } => {
let rn = get_fact(vcode, rn)?;
let rm = get_fact(vcode, rm)?;
let rn = get_fact_or_default(vcode, rn)?;
let rm = get_fact_or_default(vcode, rm)?;
let rm_extended = fail_if_missing(extend_fact(ctx, rm, extendop))?;
let sum = fail_if_missing(ctx.add(&rn, &rm_extended, 64))?;
trace!("rn = {rn:?} rm = {rm:?} rm_extended = {rm_extended:?} sum = {sum:?}");
Expand All @@ -348,12 +350,12 @@ fn check_addr<'a>(
Ok(())
}
&AMode::Unscaled { rn, simm9 } => {
let rn = get_fact(vcode, rn)?;
let rn = get_fact_or_default(vcode, rn)?;
let sum = fail_if_missing(ctx.offset(&rn, 64, simm9.value.into()))?;
check(&sum, ty)
}
&AMode::UnsignedOffset { rn, uimm12 } => {
let rn = get_fact(vcode, rn)?;
let rn = get_fact_or_default(vcode, rn)?;
// Safety: this will not overflow: `size` should be at
// most 32 or 64 for large vector ops, and the `uimm12`'s
// value is at most 4095.
Expand All @@ -371,7 +373,7 @@ fn check_addr<'a>(
Ok(())
}
&AMode::RegOffset { rn, off, .. } => {
let rn = get_fact(vcode, rn)?;
let rn = get_fact_or_default(vcode, rn)?;
let sum = fail_if_missing(ctx.offset(&rn, 64, off))?;
check(&sum, ty)
}
Expand Down Expand Up @@ -417,7 +419,7 @@ fn check_load_addr(
if !flags.checked() {
return Ok(());
}
let fact = get_fact(vcode, reg)?;
let fact = get_fact_or_default(vcode, reg)?;
let _output_fact = ctx.load(fact, ty)?;
Ok(())
}
Expand All @@ -432,7 +434,7 @@ fn check_store_addr(
if !flags.checked() {
return Ok(());
}
let fact = get_fact(vcode, reg)?;
let fact = get_fact_or_default(vcode, reg)?;
let _output_fact = ctx.store(fact, ty, None)?;
Ok(())
}
13 changes: 6 additions & 7 deletions cranelift/codegen/src/machinst/vcode.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1287,16 +1287,15 @@ impl<I: VCodeInst> VCode<I> {
op
}

/// Get the type of a VReg.
pub fn vreg_type(&self, vreg: VReg) -> Type {
self.vreg_types[vreg.vreg()]
}

/// Get the fact, if any, for a given VReg.
pub fn vreg_fact(&self, vreg: VReg) -> Option<&Fact> {
let vreg = self.resolve_vreg_alias(vreg);
match &self.facts[vreg.vreg()] {
// The vreg has a stated fact -- return that.
Some(fact) => Some(fact),
// The vreg has no fact, but maybe we can infer a minimal
// one (e.g., an integer range) from the type.
None => Fact::infer_from_type(self.vreg_types[vreg.vreg()]),
}
self.facts[vreg.vreg()].as_ref()
}

/// Does a given instruction define any facts?
Expand Down