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

Support for I128 operations in x64 backend. #2539

Merged
merged 1 commit into from
Jan 14, 2021
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
103 changes: 49 additions & 54 deletions cranelift/codegen/src/isa/x64/abi.rs
Original file line number Diff line number Diff line change
Expand Up @@ -138,42 +138,62 @@ impl ABIMachineSpec for X64ABIMachineSpec {
),
}

let intreg = in_int_reg(param.value_type);
let vecreg = in_vec_reg(param.value_type);
debug_assert!(intreg || vecreg);
debug_assert!(!(intreg && vecreg));

let (next_reg, candidate) = if intreg {
let candidate = match args_or_rets {
ArgsOrRets::Args => get_intreg_for_arg_systemv(&call_conv, next_gpr),
ArgsOrRets::Rets => get_intreg_for_retval_systemv(&call_conv, next_gpr, i),
};
debug_assert!(candidate
.map(|r| r.get_class() == RegClass::I64)
.unwrap_or(true));
(&mut next_gpr, candidate)
} else {
let candidate = match args_or_rets {
ArgsOrRets::Args => get_fltreg_for_arg_systemv(&call_conv, next_vreg),
ArgsOrRets::Rets => get_fltreg_for_retval_systemv(&call_conv, next_vreg, i),
};
debug_assert!(candidate
.map(|r| r.get_class() == RegClass::V128)
.unwrap_or(true));
(&mut next_vreg, candidate)
};

if let Some(param) = try_fill_baldrdash_reg(call_conv, param) {
assert!(intreg);
ret.push(param);
} else if let Some(reg) = candidate {
continue;
}

// Find regclass(es) of the register(s) used to store a value of this type.
let (rcs, _) = Inst::rc_for_type(param.value_type)?;
let intreg = rcs[0] == RegClass::I64;
let num_regs = rcs.len();
assert!(num_regs <= 2);
if num_regs == 2 {
assert_eq!(rcs[0], rcs[1]);
}

let mut regs: SmallVec<[RealReg; 2]> = smallvec![];
for j in 0..num_regs {
let nextreg = if intreg {
match args_or_rets {
ArgsOrRets::Args => get_intreg_for_arg_systemv(&call_conv, next_gpr + j),
ArgsOrRets::Rets => {
get_intreg_for_retval_systemv(&call_conv, next_gpr + j, i + j)
}
}
} else {
match args_or_rets {
ArgsOrRets::Args => get_fltreg_for_arg_systemv(&call_conv, next_vreg + j),
ArgsOrRets::Rets => {
get_fltreg_for_retval_systemv(&call_conv, next_vreg + j, i + j)
}
}
};
if let Some(reg) = nextreg {
regs.push(reg.to_real_reg());
} else {
regs.clear();
break;
}
}

if regs.len() > 0 {
let regs = match num_regs {
1 => ValueRegs::one(regs[0]),
2 => ValueRegs::two(regs[0], regs[1]),
_ => panic!("More than two registers unexpected"),
};
ret.push(ABIArg::Reg(
ValueRegs::one(reg.to_real_reg()),
regs,
param.value_type,
param.extension,
param.purpose,
));
*next_reg += 1;
if intreg {
next_gpr += num_regs;
} else {
next_vreg += num_regs;
}
} else {
// Compute size. Every arg takes a minimum slot of 8 bytes. (16-byte
// stack alignment happens separately after all args.)
Expand Down Expand Up @@ -658,31 +678,6 @@ impl From<StackAMode> for SyntheticAmode {
}
}

fn in_int_reg(ty: types::Type) -> bool {
match ty {
types::I8
| types::I16
| types::I32
| types::I64
| types::B1
| types::B8
| types::B16
| types::B32
| types::B64
| types::R64 => true,
types::R32 => panic!("unexpected 32-bits refs on x64!"),
_ => false,
}
}

fn in_vec_reg(ty: types::Type) -> bool {
match ty {
types::F32 | types::F64 => true,
_ if ty.is_vector() => true,
_ => false,
}
}

fn get_intreg_for_arg_systemv(call_conv: &CallConv, idx: usize) -> Option<Reg> {
match call_conv {
CallConv::Fast
Expand Down
24 changes: 23 additions & 1 deletion cranelift/codegen/src/isa/x64/inst/args.rs
Original file line number Diff line number Diff line change
Expand Up @@ -346,23 +346,35 @@ impl PrettyPrintSized for RegMem {
#[derive(Copy, Clone, PartialEq)]
pub enum AluRmiROpcode {
Add,
Adc,
Sub,
Sbb,
And,
Or,
Xor,
/// The signless, non-extending (N x N -> N, for N in {32,64}) variant.
Mul,
/// 8-bit form of And. Handled separately as we don't have full 8-bit op
/// support (we just use wider instructions). Used only with some sequences
/// with SETcc.
And8,
/// 8-bit form of Or.
Or8,
}

impl fmt::Debug for AluRmiROpcode {
fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result {
let name = match self {
AluRmiROpcode::Add => "add",
AluRmiROpcode::Adc => "adc",
AluRmiROpcode::Sub => "sub",
AluRmiROpcode::Sbb => "sbb",
AluRmiROpcode::And => "and",
AluRmiROpcode::Or => "or",
AluRmiROpcode::Xor => "xor",
AluRmiROpcode::Mul => "imul",
AluRmiROpcode::And8 => "and",
AluRmiROpcode::Or8 => "or",
};
write!(fmt, "{}", name)
}
Expand All @@ -374,6 +386,16 @@ impl fmt::Display for AluRmiROpcode {
}
}

impl AluRmiROpcode {
/// Is this a special-cased 8-bit ALU op?
pub fn is_8bit(self) -> bool {
match self {
AluRmiROpcode::And8 | AluRmiROpcode::Or8 => true,
_ => false,
}
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

It seems preferable to me to just have AluRmiROpcode::And and avoid this special-casing if possible. Do you agree? If yes, I wonder if there is a way to standardize register sizes throughout lower.rs using OperandSize and trying to clarify/simplify all of the special logic related to variables like is64, is_64, is_8bit, etc.

Copy link
Member Author

Choose a reason for hiding this comment

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

In general I would agree, except that we don't use 8-bit ops elsewhere for CLIF-level 8-bit instructions so I went for the limited-in-scope approach here. I'm totally open to a future refactor that centralizes OperandSize for all instructions, though -- that would remove a lot of the ad-hoc conditionals, as you note!


#[derive(Clone, PartialEq)]
pub enum UnaryRmROpcode {
/// Bit-scan reverse.
Expand Down Expand Up @@ -1010,7 +1032,7 @@ impl fmt::Display for ExtMode {
}

/// These indicate the form of a scalar shift/rotate: left, signed right, unsigned right.
#[derive(Clone)]
#[derive(Clone, Copy)]
pub enum ShiftKind {
ShiftLeft,
/// Inserts zeros in the most significant bits.
Expand Down
Loading