Skip to content

Commit

Permalink
Cranelift(x64): provide better panic messages for `Gpr::new(reg).unwr…
Browse files Browse the repository at this point in the history 
…ap()` et al (#9005)

* Cranelift(x64): provide better panic messages for `Gpr::new(reg).unwrap()` et al

Co-Authored-By: Jamey Sharp <jsharp@fastly.com>

* Fix compile error due to bad mechanical replacement

---------

Co-authored-by: Jamey Sharp <jsharp@fastly.com>
  • Loading branch information
@fitzgen @jameysharp
fitzgen and jameysharp authored Jul 24, 2024
1 parent 488e505 commit bc3d612
Show file tree
Hide file tree
Showing 9 changed files with 336 additions and 341 deletions.
20 changes: 10 additions & 10 deletions cranelift/codegen/src/isa/x64/encoding/evex.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -552,8 +552,8 @@ mod tests {
regs::xmm9(),
Amode::ImmRegRegShift {
simm32: 0,
base: Gpr::new(regs::rbx()).unwrap(),
index: Gpr::new(regs::rsi()).unwrap(),
base: Gpr::unwrap_new(regs::rbx()),
index: Gpr::unwrap_new(regs::rsi()),
shift: 3,
flags: MemFlags::trusted(),
}
Expand All @@ -565,8 +565,8 @@ mod tests {
regs::xmm13(),
Amode::ImmRegRegShift {
simm32: 1,
base: Gpr::new(regs::r11()).unwrap(),
index: Gpr::new(regs::rdi()).unwrap(),
base: Gpr::unwrap_new(regs::r11()),
index: Gpr::unwrap_new(regs::rdi()),
shift: 2,
flags: MemFlags::trusted(),
}
Expand All @@ -580,8 +580,8 @@ mod tests {
regs::xmm5(),
Amode::ImmRegRegShift {
simm32: 128,
base: Gpr::new(regs::rsp()).unwrap(),
index: Gpr::new(regs::r10()).unwrap(),
base: Gpr::unwrap_new(regs::rsp()),
index: Gpr::unwrap_new(regs::r10()),
shift: 1,
flags: MemFlags::trusted(),
}
Expand All @@ -593,8 +593,8 @@ mod tests {
regs::xmm6(),
Amode::ImmRegRegShift {
simm32: 112,
base: Gpr::new(regs::rbp()).unwrap(),
index: Gpr::new(regs::r13()).unwrap(),
base: Gpr::unwrap_new(regs::rbp()),
index: Gpr::unwrap_new(regs::r13()),
shift: 0,
flags: MemFlags::trusted(),
}
Expand All @@ -606,8 +606,8 @@ mod tests {
regs::xmm7(),
Amode::ImmRegRegShift {
simm32: 0,
base: Gpr::new(regs::rbp()).unwrap(),
index: Gpr::new(regs::r13()).unwrap(),
base: Gpr::unwrap_new(regs::rbp()),
index: Gpr::unwrap_new(regs::r13()),
shift: 0,
flags: MemFlags::trusted(),
}
Expand Down
4 changes: 2 additions & 2 deletions cranelift/codegen/src/isa/x64/encoding/vex.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -465,8 +465,8 @@ mod tests {
let dst = regs::xmm2().to_real_reg().unwrap().hw_enc();
let src1 = regs::xmm1().to_real_reg().unwrap().hw_enc();
let src2 = Amode::ImmRegRegShift {
base: Gpr::new(regs::rax()).unwrap(),
index: Gpr::new(regs::r13()).unwrap(),
base: Gpr::unwrap_new(regs::rax()),
index: Gpr::unwrap_new(regs::r13()),
flags: MemFlags::trusted(),
simm32: 100,
shift: 2,
Expand Down
74 changes: 68 additions & 6 deletions cranelift/codegen/src/isa/x64/inst/args.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,21 @@ macro_rules! newtype_of_reg {
}
}

/// Like `Self::new(r).unwrap()` but with a better panic message on
/// failure.
pub fn unwrap_new($check_reg: Reg) -> Self {
if $check {
Self($check_reg)
} else {
panic!(
"cannot construct {} from register {:?} with register class {:?}",
stringify!($newtype_reg),
$check_reg,
$check_reg.class(),
)
}
}

/// Get this newtype's underlying `Reg`.
pub fn to_reg(self) -> Reg {
self.0
Expand Down Expand Up @@ -154,8 +169,26 @@ macro_rules! newtype_of_reg {
None
}
}
RegMem::Reg { reg: $check_reg } if $check => Some(Self(rm)),
RegMem::Reg { reg: _ } => None,
RegMem::Reg { reg } => Some($newtype_reg::new(reg)?.into()),
}
}

/// Like `Self::new(rm).unwrap()` but with better panic messages
/// in case of failure.
pub fn unwrap_new(rm: RegMem) -> Self {
match rm {
RegMem::Mem { addr } => {
$(
if $aligned && !addr.aligned() {
panic!(
"cannot create {} from an unaligned memory address: {addr:?}",
stringify!($newtype_reg_mem),
);
}
)?
Self(RegMem::Mem { addr })
}
RegMem::Reg { reg } => $newtype_reg::unwrap_new(reg).into(),
}
}

Expand Down Expand Up @@ -218,8 +251,29 @@ macro_rules! newtype_of_reg {
None
}
}
RegMemImm::Reg { reg: $check_reg } if $check => Some(Self(rmi)),
RegMemImm::Reg { reg: _ } => None,
RegMemImm::Reg { reg } => Some($newtype_reg::new(reg)?.into()),
}
}

/// Like `Self::new(rmi).unwrap()` but with better panic
/// messages in case of failure.
pub fn unwrap_new(rmi: RegMemImm) -> Self {
match rmi {
RegMemImm::Imm { .. } => Self(rmi),
RegMemImm::Mem { addr } => {
$(
if $aligned_imm && !addr.aligned() {
panic!(
"cannot construct {} from unaligned memory address: {:?}",
stringify!($newtype_reg_mem_imm),
addr,
);
}
)?
Self(RegMemImm::Mem { addr })

}
RegMemImm::Reg { reg } => $newtype_reg::unwrap_new(reg).into(),
}
}

Expand Down Expand Up @@ -260,8 +314,16 @@ macro_rules! newtype_of_reg {
pub fn new(imm8_reg: Imm8Reg) -> Option<Self> {
match imm8_reg {
Imm8Reg::Imm8 { .. } => Some(Self(imm8_reg)),
Imm8Reg::Reg { reg: $check_reg } if $check => Some(Self(imm8_reg)),
Imm8Reg::Reg { reg: _ } => None,
Imm8Reg::Reg { reg } => Some($newtype_reg::new(reg)?.into()),
}
}

/// Like `Self::new(imm8_reg).unwrap()` but with better panic
/// messages on failure.
pub fn unwrap_new(imm8_reg: Imm8Reg) -> Self {
match imm8_reg {
Imm8Reg::Imm8 { .. } => Self(imm8_reg),
Imm8Reg::Reg { reg } => $newtype_reg::unwrap_new(reg).into(),
}
}

Expand Down
28 changes: 14 additions & 14 deletions cranelift/codegen/src/isa/x64/inst/emit.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ fn emit_signed_cvt(
op,
dst,
src1: dst.to_reg(),
src2: GprMem::new(RegMem::reg(src)).unwrap(),
src2: GprMem::unwrap_new(RegMem::reg(src)),
src2_size: OperandSize::Size64,
}
.emit(sink, info, state);
Expand Down Expand Up @@ -788,18 +788,18 @@ pub(crate) fn emit(
DivSignedness::Signed,
TrapCode::IntegerDivisionByZero,
RegMem::reg(divisor),
Gpr::new(regs::rax()).unwrap(),
Writable::from_reg(Gpr::new(regs::rax()).unwrap()),
Gpr::unwrap_new(regs::rax()),
Writable::from_reg(Gpr::unwrap_new(regs::rax())),
),
_ => Inst::div(
size,
DivSignedness::Signed,
TrapCode::IntegerDivisionByZero,
RegMem::reg(divisor),
Gpr::new(regs::rax()).unwrap(),
Gpr::new(regs::rdx()).unwrap(),
Writable::from_reg(Gpr::new(regs::rax()).unwrap()),
Writable::from_reg(Gpr::new(regs::rdx()).unwrap()),
Gpr::unwrap_new(regs::rax()),
Gpr::unwrap_new(regs::rdx()),
Writable::from_reg(Gpr::unwrap_new(regs::rax())),
Writable::from_reg(Gpr::unwrap_new(regs::rdx())),
),
};
inst.emit(sink, info, state);
Expand Down Expand Up @@ -884,15 +884,15 @@ pub(crate) fn emit(
Inst::MovFromPReg { src, dst } => {
let src: Reg = (*src).into();
debug_assert!([regs::rsp(), regs::rbp(), regs::pinned_reg()].contains(&src));
let src = Gpr::new(src).unwrap();
let src = Gpr::unwrap_new(src);
let size = OperandSize::Size64;
let dst = WritableGpr::from_writable_reg(dst.to_writable_reg()).unwrap();
Inst::MovRR { size, src, dst }.emit(sink, info, state);
}

Inst::MovToPReg { src, dst } => {
let src = src.to_reg();
let src = Gpr::new(src).unwrap();
let src = Gpr::unwrap_new(src);
let dst: Reg = (*dst).into();
debug_assert!([regs::rsp(), regs::rbp(), regs::pinned_reg()].contains(&dst));
let dst = WritableGpr::from_writable_reg(Writable::from_reg(dst)).unwrap();
Expand Down Expand Up @@ -1866,8 +1866,8 @@ pub(crate) fn emit(
ExtMode::LQ,
RegMem::mem(Amode::imm_reg_reg_shift(
0,
Gpr::new(tmp1.to_reg()).unwrap(),
Gpr::new(idx).unwrap(),
Gpr::unwrap_new(tmp1.to_reg()),
Gpr::unwrap_new(idx),
2,
)),
tmp2,
Expand Down Expand Up @@ -1939,7 +1939,7 @@ pub(crate) fn emit(
emit(
&Inst::XmmUnaryRmRUnaligned {
op: *op,
src: XmmMem::new(src.clone().into()).unwrap(),
src: XmmMem::unwrap_new(src.clone().into()),
dst: *dst,
},
sink,
Expand Down Expand Up @@ -2097,7 +2097,7 @@ pub(crate) fn emit(
op: *op,
dst: *dst,
src1: *src1,
src2: XmmMem::new(src2.clone().to_reg_mem()).unwrap(),
src2: XmmMem::unwrap_new(src2.clone().to_reg_mem()),
},
sink,
info,
Expand Down Expand Up @@ -3397,7 +3397,7 @@ pub(crate) fn emit(
let inst = Inst::shift_r(
OperandSize::Size64,
ShiftKind::ShiftRightLogical,
Imm8Gpr::new(Imm8Reg::Imm8 { imm: 1 }).unwrap(),
Imm8Gpr::unwrap_new(Imm8Reg::Imm8 { imm: 1 }),
tmp_gpr1.to_reg(),
tmp_gpr1,
);
Expand Down
Loading

0 comments on commit bc3d612

Please sign in to comment.