Address review comments;

This commit is contained in:
Benjamin Bouvier
2020-07-16 15:19:38 +02:00
parent 5a55646fc3
commit bab337fc32
6 changed files with 213 additions and 127 deletions

View File

@@ -282,32 +282,32 @@ impl fmt::Debug for AluRmiROpcode {
}
}
impl ToString for AluRmiROpcode {
fn to_string(&self) -> String {
format!("{:?}", self)
impl fmt::Display for AluRmiROpcode {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
fmt::Debug::fmt(self, f)
}
}
#[derive(Clone, PartialEq)]
pub enum ReadOnlyGprRmROpcode {
pub enum UnaryRmROpcode {
/// Bit-scan reverse.
Bsr,
/// Bit-scan forward.
Bsf,
}
impl fmt::Debug for ReadOnlyGprRmROpcode {
impl fmt::Debug for UnaryRmROpcode {
fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result {
match self {
ReadOnlyGprRmROpcode::Bsr => write!(fmt, "bsr"),
ReadOnlyGprRmROpcode::Bsf => write!(fmt, "bsf"),
UnaryRmROpcode::Bsr => write!(fmt, "bsr"),
UnaryRmROpcode::Bsf => write!(fmt, "bsf"),
}
}
}
impl ToString for ReadOnlyGprRmROpcode {
fn to_string(&self) -> String {
format!("{:?}", self)
impl fmt::Display for UnaryRmROpcode {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
fmt::Debug::fmt(self, f)
}
}
@@ -468,9 +468,9 @@ impl fmt::Debug for SseOpcode {
}
}
impl ToString for SseOpcode {
fn to_string(&self) -> String {
format!("{:?}", self)
impl fmt::Display for SseOpcode {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
fmt::Debug::fmt(self, f)
}
}
@@ -519,18 +519,20 @@ impl fmt::Debug for ExtMode {
}
}
impl ToString for ExtMode {
fn to_string(&self) -> String {
format!("{:?}", self)
impl fmt::Display for ExtMode {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
fmt::Debug::fmt(self, f)
}
}
/// These indicate the form of a scalar shift: left, signed right, unsigned right.
/// These indicate the form of a scalar shift/rotate: left, signed right, unsigned right.
#[derive(Clone)]
pub enum ShiftKind {
Left,
RightZ,
RightS,
ShiftLeft,
/// Inserts zeros in the most significant bits.
ShiftRightLogical,
/// Replicates the sign bit in the most significant bits.
ShiftRightArithmetic,
RotateLeft,
RotateRight,
}
@@ -538,9 +540,9 @@ pub enum ShiftKind {
impl fmt::Debug for ShiftKind {
fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result {
let name = match self {
ShiftKind::Left => "shl",
ShiftKind::RightZ => "shr",
ShiftKind::RightS => "sar",
ShiftKind::ShiftLeft => "shl",
ShiftKind::ShiftRightLogical => "shr",
ShiftKind::ShiftRightArithmetic => "sar",
ShiftKind::RotateLeft => "rol",
ShiftKind::RotateRight => "ror",
};
@@ -548,9 +550,34 @@ impl fmt::Debug for ShiftKind {
}
}
impl ToString for ShiftKind {
fn to_string(&self) -> String {
format!("{:?}", self)
impl fmt::Display for ShiftKind {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
fmt::Debug::fmt(self, f)
}
}
/// What kind of division or remainer instruction this is?
#[derive(Clone)]
pub enum DivOrRemKind {
SignedDiv,
UnsignedDiv,
SignedRem,
UnsignedRem,
}
impl DivOrRemKind {
pub(crate) fn is_signed(&self) -> bool {
match self {
DivOrRemKind::SignedDiv | DivOrRemKind::SignedRem => true,
_ => false,
}
}
pub(crate) fn is_div(&self) -> bool {
match self {
DivOrRemKind::SignedDiv | DivOrRemKind::UnsignedDiv => true,
_ => false,
}
}
}
@@ -665,9 +692,9 @@ impl fmt::Debug for CC {
}
}
impl ToString for CC {
fn to_string(&self) -> String {
format!("{:?}", self)
impl fmt::Display for CC {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
fmt::Debug::fmt(self, f)
}
}

View File

@@ -556,7 +556,7 @@ pub(crate) fn emit(
}
}
Inst::ReadOnly_Gpr_Rm_R { size, op, src, dst } => {
Inst::UnaryRmR { size, op, src, dst } => {
let (prefix, rex_flags) = match size {
2 => (LegacyPrefix::_66, RexFlags::clear_w()),
4 => (LegacyPrefix::None, RexFlags::clear_w()),
@@ -565,8 +565,8 @@ pub(crate) fn emit(
};
let (opcode, num_opcodes) = match op {
ReadOnlyGprRmROpcode::Bsr => (0x0fbd, 2),
ReadOnlyGprRmROpcode::Bsf => (0x0fbc, 2),
UnaryRmROpcode::Bsr => (0x0fbd, 2),
UnaryRmROpcode::Bsf => (0x0fbc, 2),
};
match src {
@@ -661,8 +661,7 @@ pub(crate) fn emit(
}
Inst::CheckedDivOrRemSeq {
is_div,
is_signed,
kind,
size,
divisor,
loc,
@@ -704,7 +703,7 @@ pub(crate) fn emit(
let inst = Inst::trap_if(CC::Z, TrapCode::IntegerDivisionByZero, *loc);
inst.emit(sink, flags, state);
let (do_op, done_label) = if *is_signed {
let (do_op, done_label) = if kind.is_signed() {
// Now check if the divisor is -1.
let inst = Inst::cmp_rmi_r(*size, RegMemImm::imm(0xffffffff), *divisor);
inst.emit(sink, flags, state);
@@ -715,7 +714,7 @@ pub(crate) fn emit(
one_way_jmp(sink, CC::NZ, do_op);
// Here, divisor == -1.
if !*is_div {
if !kind.is_div() {
// x % -1 = 0; put the result into the destination, $rdx.
let done_label = sink.get_label();
@@ -756,7 +755,7 @@ pub(crate) fn emit(
}
// Fill in the high parts:
if *is_signed {
if kind.is_signed() {
// sign-extend the sign-bit of rax into rdx, for signed opcodes.
let inst = Inst::sign_extend_rax_to_rdx(*size);
inst.emit(sink, flags, state);
@@ -766,7 +765,7 @@ pub(crate) fn emit(
inst.emit(sink, flags, state);
}
let inst = Inst::div(*size, *is_signed, RegMem::reg(*divisor), *loc);
let inst = Inst::div(*size, kind.is_signed(), RegMem::reg(*divisor), *loc);
inst.emit(sink, flags, state);
// Lowering takes care of moving the result back into the right register, see comment
@@ -1047,9 +1046,9 @@ pub(crate) fn emit(
let subopcode = match kind {
ShiftKind::RotateLeft => 0,
ShiftKind::RotateRight => 1,
ShiftKind::Left => 4,
ShiftKind::RightZ => 5,
ShiftKind::RightS => 7,
ShiftKind::ShiftLeft => 4,
ShiftKind::ShiftRightLogical => 5,
ShiftKind::ShiftRightArithmetic => 7,
};
let rex = if *is_64 {
@@ -1550,6 +1549,7 @@ pub(crate) fn emit(
srcloc,
} => {
// The full address can be encoded in the register, with a relocation.
// Generates: movabsq $name, %dst
let enc_dst = int_reg_enc(dst.to_reg());
sink.put1(0x48 | ((enc_dst >> 3) & 1));
sink.put1(0xB8 | (enc_dst & 7));

View File

@@ -1227,15 +1227,15 @@ fn test_x64_emit() {
));
// ========================================================
// ReadOnly_Gpr_Rm_R
// UnaryRmR
insns.push((
Inst::read_only_gpr_rm_r(4, ReadOnlyGprRmROpcode::Bsr, RegMem::reg(rsi), w_rdi),
Inst::unary_rm_r(4, UnaryRmROpcode::Bsr, RegMem::reg(rsi), w_rdi),
"0FBDFE",
"bsrl %esi, %edi",
));
insns.push((
Inst::read_only_gpr_rm_r(8, ReadOnlyGprRmROpcode::Bsr, RegMem::reg(r15), w_rax),
Inst::unary_rm_r(8, UnaryRmROpcode::Bsr, RegMem::reg(r15), w_rax),
"490FBDC7",
"bsrq %r15, %rax",
));
@@ -2303,107 +2303,107 @@ fn test_x64_emit() {
// ========================================================
// Shift_R
insns.push((
Inst::shift_r(false, ShiftKind::Left, None, w_rdi),
Inst::shift_r(false, ShiftKind::ShiftLeft, None, w_rdi),
"D3E7",
"shll %cl, %edi",
));
insns.push((
Inst::shift_r(false, ShiftKind::Left, None, w_r12),
Inst::shift_r(false, ShiftKind::ShiftLeft, None, w_r12),
"41D3E4",
"shll %cl, %r12d",
));
insns.push((
Inst::shift_r(false, ShiftKind::Left, Some(2), w_r8),
Inst::shift_r(false, ShiftKind::ShiftLeft, Some(2), w_r8),
"41C1E002",
"shll $2, %r8d",
));
insns.push((
Inst::shift_r(false, ShiftKind::Left, Some(31), w_r13),
Inst::shift_r(false, ShiftKind::ShiftLeft, Some(31), w_r13),
"41C1E51F",
"shll $31, %r13d",
));
insns.push((
Inst::shift_r(true, ShiftKind::Left, None, w_r13),
Inst::shift_r(true, ShiftKind::ShiftLeft, None, w_r13),
"49D3E5",
"shlq %cl, %r13",
));
insns.push((
Inst::shift_r(true, ShiftKind::Left, None, w_rdi),
Inst::shift_r(true, ShiftKind::ShiftLeft, None, w_rdi),
"48D3E7",
"shlq %cl, %rdi",
));
insns.push((
Inst::shift_r(true, ShiftKind::Left, Some(2), w_r8),
Inst::shift_r(true, ShiftKind::ShiftLeft, Some(2), w_r8),
"49C1E002",
"shlq $2, %r8",
));
insns.push((
Inst::shift_r(true, ShiftKind::Left, Some(3), w_rbx),
Inst::shift_r(true, ShiftKind::ShiftLeft, Some(3), w_rbx),
"48C1E303",
"shlq $3, %rbx",
));
insns.push((
Inst::shift_r(true, ShiftKind::Left, Some(63), w_r13),
Inst::shift_r(true, ShiftKind::ShiftLeft, Some(63), w_r13),
"49C1E53F",
"shlq $63, %r13",
));
insns.push((
Inst::shift_r(false, ShiftKind::RightZ, None, w_rdi),
Inst::shift_r(false, ShiftKind::ShiftRightLogical, None, w_rdi),
"D3EF",
"shrl %cl, %edi",
));
insns.push((
Inst::shift_r(false, ShiftKind::RightZ, Some(2), w_r8),
Inst::shift_r(false, ShiftKind::ShiftRightLogical, Some(2), w_r8),
"41C1E802",
"shrl $2, %r8d",
));
insns.push((
Inst::shift_r(false, ShiftKind::RightZ, Some(31), w_r13),
Inst::shift_r(false, ShiftKind::ShiftRightLogical, Some(31), w_r13),
"41C1ED1F",
"shrl $31, %r13d",
));
insns.push((
Inst::shift_r(true, ShiftKind::RightZ, None, w_rdi),
Inst::shift_r(true, ShiftKind::ShiftRightLogical, None, w_rdi),
"48D3EF",
"shrq %cl, %rdi",
));
insns.push((
Inst::shift_r(true, ShiftKind::RightZ, Some(2), w_r8),
Inst::shift_r(true, ShiftKind::ShiftRightLogical, Some(2), w_r8),
"49C1E802",
"shrq $2, %r8",
));
insns.push((
Inst::shift_r(true, ShiftKind::RightZ, Some(63), w_r13),
Inst::shift_r(true, ShiftKind::ShiftRightLogical, Some(63), w_r13),
"49C1ED3F",
"shrq $63, %r13",
));
insns.push((
Inst::shift_r(false, ShiftKind::RightS, None, w_rdi),
Inst::shift_r(false, ShiftKind::ShiftRightArithmetic, None, w_rdi),
"D3FF",
"sarl %cl, %edi",
));
insns.push((
Inst::shift_r(false, ShiftKind::RightS, Some(2), w_r8),
Inst::shift_r(false, ShiftKind::ShiftRightArithmetic, Some(2), w_r8),
"41C1F802",
"sarl $2, %r8d",
));
insns.push((
Inst::shift_r(false, ShiftKind::RightS, Some(31), w_r13),
Inst::shift_r(false, ShiftKind::ShiftRightArithmetic, Some(31), w_r13),
"41C1FD1F",
"sarl $31, %r13d",
));
insns.push((
Inst::shift_r(true, ShiftKind::RightS, None, w_rdi),
Inst::shift_r(true, ShiftKind::ShiftRightArithmetic, None, w_rdi),
"48D3FF",
"sarq %cl, %rdi",
));
insns.push((
Inst::shift_r(true, ShiftKind::RightS, Some(2), w_r8),
Inst::shift_r(true, ShiftKind::ShiftRightArithmetic, Some(2), w_r8),
"49C1F802",
"sarq $2, %r8",
));
insns.push((
Inst::shift_r(true, ShiftKind::RightS, Some(63), w_r13),
Inst::shift_r(true, ShiftKind::ShiftRightArithmetic, Some(63), w_r13),
"49C1FD3F",
"sarq $63, %r13",
));

View File

@@ -51,9 +51,9 @@ pub enum Inst {
},
/// Instructions on GPR that only read src and defines dst (dst is not modified): bsr, etc.
ReadOnly_Gpr_Rm_R {
UnaryRmR {
size: u8, // 2, 4 or 8
op: ReadOnlyGprRmROpcode,
op: UnaryRmROpcode,
src: RegMem,
dst: Writable<Reg>,
},
@@ -66,7 +66,7 @@ pub enum Inst {
loc: SourceLoc,
},
/// The high result of a (un)signed multiply: imul/mul using RAX:RDX.
/// The high bits (RDX) of a (un)signed multiply: RDX:RAX := RAX * rhs.
MulHi { size: u8, signed: bool, rhs: RegMem },
/// A synthetic sequence to implement the right inline checks for remainder and division,
@@ -77,10 +77,11 @@ pub enum Inst {
/// instruction.
///
/// Note: %rdx is marked as modified by this instruction, to avoid an early clobber problem
/// with the temporary and divisor. Make sure to zero %rdx right before this instruction!
/// with the temporary and divisor registers. Make sure to zero %rdx right before this
/// instruction, or you might run into regalloc failures where %rdx is live before its first
/// def!
CheckedDivOrRemSeq {
is_div: bool,
is_signed: bool,
kind: DivOrRemKind,
size: u8,
divisor: Reg,
tmp: Option<Writable<Reg>>,
@@ -283,7 +284,7 @@ pub enum Inst {
/// An instruction that will always trigger the illegal instruction exception.
Ud2 { trap_info: (SourceLoc, TrapCode) },
/// Loads an external symbol in a register, with a relocation.
/// Loads an external symbol in a register, with a relocation: movabsq $name, dst
LoadExtName {
dst: Writable<Reg>,
name: Box<ExternalName>,
@@ -326,15 +327,15 @@ impl Inst {
}
}
pub(crate) fn read_only_gpr_rm_r(
pub(crate) fn unary_rm_r(
size: u8,
op: ReadOnlyGprRmROpcode,
op: UnaryRmROpcode,
src: RegMem,
dst: Writable<Reg>,
) -> Self {
debug_assert!(dst.to_reg().get_class() == RegClass::I64);
debug_assert!(size == 8 || size == 4 || size == 2);
Self::ReadOnly_Gpr_Rm_R { size, op, src, dst }
Self::UnaryRmR { size, op, src, dst }
}
pub(crate) fn div(size: u8, signed: bool, divisor: RegMem, loc: SourceLoc) -> Inst {
@@ -667,7 +668,7 @@ impl ShowWithRRU for Inst {
show_ireg_sized(dst.to_reg(), mb_rru, sizeLQ(*is_64)),
),
Inst::ReadOnly_Gpr_Rm_R { src, dst, op, size } => format!(
Inst::UnaryRmR { src, dst, op, size } => format!(
"{} {}, {}",
ljustify2(op.to_string(), suffixBWLQ(*size)),
src.show_rru_sized(mb_rru, *size),
@@ -700,15 +701,18 @@ impl ShowWithRRU for Inst {
rhs.show_rru_sized(mb_rru, *size)
),
Inst::CheckedDivOrRemSeq {
is_div,
is_signed,
kind,
size,
divisor,
..
} => format!(
"{}{} $rax:$rdx, {}",
if *is_signed { "s" } else { "u" },
if *is_div { "div " } else { "rem " },
"{} $rax:$rdx, {}",
match kind {
DivOrRemKind::SignedDiv => "sdiv",
DivOrRemKind::UnsignedDiv => "udiv",
DivOrRemKind::SignedRem => "srem",
DivOrRemKind::UnsignedRem => "urem",
},
show_ireg_sized(*divisor, mb_rru, *size),
),
Inst::SignExtendRaxRdx { size } => match size {
@@ -942,7 +946,7 @@ fn x64_get_regs(inst: &Inst, collector: &mut RegUsageCollector) {
collector.add_use(regs::rax());
collector.add_mod(Writable::from_reg(regs::rdx()));
}
Inst::ReadOnly_Gpr_Rm_R { src, dst, .. } | Inst::XMM_Mov_RM_R { src, dst, .. } => {
Inst::UnaryRmR { src, dst, .. } | Inst::XMM_Mov_RM_R { src, dst, .. } => {
src.get_regs_as_uses(collector);
collector.add_def(*dst);
}
@@ -1141,7 +1145,7 @@ fn x64_map_regs<RUM: RegUsageMapper>(inst: &mut Inst, mapper: &RUM) {
ref mut dst,
..
}
| Inst::ReadOnly_Gpr_Rm_R {
| Inst::UnaryRmR {
ref mut src,
ref mut dst,
..