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

s390x: Basic support for IaddIfcout #3012

Merged
merged 1 commit into from
Jun 22, 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
44 changes: 39 additions & 5 deletions cranelift/codegen/src/isa/s390x/lower.rs
Original file line number Diff line number Diff line change
Expand Up @@ -962,6 +962,19 @@ fn lower_insn_to_regs<C: LowerCtx<I = Inst>>(
ctx.emit(Inst::AluRRR { alu_op, rd, rn, rm });
}
}
Opcode::IaddIfcout => {
// This only supports the operands emitted by dynamic_addr.
let ty = ty.unwrap();
assert!(ty == types::I32 || ty == types::I64);
let alu_op = choose_32_64(ty, ALUOp::Add32, ALUOp::Add64);
let rd = get_output_reg(ctx, outputs[0]).only_reg().unwrap();
let rn = put_input_in_reg(ctx, inputs[0], NarrowValueMode::None);
let imm = input_matches_uimm32(ctx, inputs[1]).unwrap();
ctx.emit(Inst::gen_move(rd, rn, ty));
// Note that this will emit AL(G)FI, which sets the condition
// code to indicate an (unsigned) carry bit.
ctx.emit(Inst::AluRUImm32 { alu_op, rd, imm });
}

Opcode::UaddSat | Opcode::SaddSat => unimplemented!(),
Opcode::UsubSat | Opcode::SsubSat => unimplemented!(),
Expand Down Expand Up @@ -2565,17 +2578,39 @@ fn lower_insn_to_regs<C: LowerCtx<I = Inst>>(

Opcode::Trapif => {
let condcode = ctx.data(insn).cond_code().unwrap();
let cond = Cond::from_intcc(condcode);
let mut cond = Cond::from_intcc(condcode);
let is_signed = condcode_is_signed(condcode);

// Verification ensures that the input is always a single-def ifcmp.
let cmp_insn = ctx
.get_input_as_source_or_const(inputs[0].insn, inputs[0].input)
.inst
.unwrap()
.0;
debug_assert_eq!(ctx.data(cmp_insn).opcode(), Opcode::Ifcmp);
lower_icmp_to_flags(ctx, cmp_insn, is_signed, true);
if ctx.data(cmp_insn).opcode() == Opcode::IaddIfcout {
// The flags must not have been clobbered by any other instruction between the
// iadd_ifcout and this instruction, as verified by the CLIF validator; so we
// can simply rely on the condition code here.
//
// IaddIfcout is implemented via a ADD LOGICAL instruction, which sets the
// the condition code as follows:
// 0 Result zero; no carry
// 1 Result not zero; no carry
// 2 Result zero; carry
// 3 Result not zero; carry
// This means "carry" corresponds to condition code 2 or 3, i.e.
// a condition mask of 2 | 1.
//
// As this does not match any of the encodings used with a normal integer
// comparsion, this cannot be represented by any IntCC value. We need to
// remap the IntCC::UnsignedGreaterThan value that we have here as result
// of the unsigned_add_overflow_condition call to the correct mask.
assert!(condcode == IntCC::UnsignedGreaterThan);
cond = Cond::from_mask(2 | 1);
Copy link
Member

Choose a reason for hiding this comment

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

Could we add a comment here noting what these constants are (which condition flags) and how the combination of the add instruction generated by IaddIfcout and the condcode here will properly trap?

Perhaps also a note in unsigned_add_overflow_condition() would be helpful in order to describe how the condition code is actually specially handled in the one place this is known to be used.

} else {
// Verification ensures that the input is always a single-def ifcmp
debug_assert_eq!(ctx.data(cmp_insn).opcode(), Opcode::Ifcmp);
lower_icmp_to_flags(ctx, cmp_insn, is_signed, true);
}

let trap_code = ctx.data(insn).trap_code().unwrap();
ctx.emit_safepoint(Inst::TrapIf { trap_code, cond });
Expand Down Expand Up @@ -2894,7 +2929,6 @@ fn lower_insn_to_regs<C: LowerCtx<I = Inst>>(
| Opcode::IaddCin
| Opcode::IaddIfcin
| Opcode::IaddCout
| Opcode::IaddIfcout
| Opcode::IaddCarry
| Opcode::IaddIfcarry
| Opcode::IsubBin
Expand Down
7 changes: 6 additions & 1 deletion cranelift/codegen/src/isa/s390x/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,12 @@ impl MachBackend for S390xBackend {
}

fn unsigned_add_overflow_condition(&self) -> IntCC {
unimplemented!()
// The ADD LOGICAL family of instructions set the condition code
// differently from normal comparisons, in a way that cannot be
// represented by any of the standard IntCC values. So we use a
// dummy value here, which gets remapped to the correct condition
// code mask during lowering.
IntCC::UnsignedGreaterThan
}

fn unsigned_sub_overflow_condition(&self) -> IntCC {
Expand Down