diff --git a/cranelift/codegen/src/isa/s390x/inst.isle b/cranelift/codegen/src/isa/s390x/inst.isle index 7fad425ae0b3..45fc2d3c0339 100644 --- a/cranelift/codegen/src/isa/s390x/inst.isle +++ b/cranelift/codegen/src/isa/s390x/inst.isle @@ -942,21 +942,6 @@ (cond Cond) (trap_code TrapCode)) - ;; A one-way conditional branch, invisible to the CFG processing; used *only* as part of - ;; straight-line sequences in code to be emitted. - ;; - ;; In more detail: - ;; - This branch is lowered to a branch at the machine-code level, but does not end a basic - ;; block, and does not create edges in the CFG seen by regalloc. - ;; - Thus, it is *only* valid to use as part of a single-in, single-out sequence that is - ;; lowered from a single CLIF instruction. For example, certain arithmetic operations may - ;; use these branches to handle certain conditions, such as overflows, traps, etc. - ;; - ;; See, e.g., the lowering of `trapif` (conditional trap) for an example. - (OneWayCondBr - (target MachLabel) - (cond Cond)) - ;; An indirect branch through a register, augmented with set of all ;; possible successors. (IndirectBr @@ -974,7 +959,14 @@ ;; Jump-table sequence, as one compound instruction (see note in lower.rs ;; for rationale). (JTSequence + ;; The index into the list of targets. (ridx Reg) + ;; The default target. + (default MachLabel) + ;; The condition for taking the default target, based on flags + ;; when this instruction executes. + (default_cond Cond) + ;; All other targets. (targets BoxVecMachLabel)) ;; Stack probe loop sequence, as one compound instruction. @@ -2702,16 +2694,10 @@ (rule (cond_br taken not_taken cond) (ConsumesFlags.ConsumesFlagsSideEffect (MInst.CondBr taken not_taken cond))) -;; Helper for emitting `MInst.OneWayCondBr` instructions. -(decl oneway_cond_br (MachLabel Cond) ConsumesFlags) -(rule (oneway_cond_br dest cond) - (ConsumesFlags.ConsumesFlagsSideEffect (MInst.OneWayCondBr dest cond))) - ;; Helper for emitting `MInst.JTSequence` instructions. -(decl jt_sequence (Reg BoxVecMachLabel) SideEffectNoResult) -(rule (jt_sequence ridx targets) - (SideEffectNoResult.Inst (MInst.JTSequence ridx targets))) - +(decl jt_sequence (Reg MachLabel Cond BoxVecMachLabel) ConsumesFlags) +(rule (jt_sequence ridx default default_cond targets) + (ConsumesFlags.ConsumesFlagsSideEffect (MInst.JTSequence ridx default default_cond targets))) ;; Helpers for instruction sequences ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; @@ -3324,6 +3310,14 @@ (decl bool (ProducesFlags Cond) ProducesBool) (rule (bool producer cond) (ProducesBool.ProducesBool producer cond)) +;; Get the producer from a ProducesBool. +(decl bool_producer (ProducesBool) ProducesFlags) +(rule (bool_producer (ProducesBool.ProducesBool producer _)) producer) + +;; Get the condition from a ProducesBool. +(decl bool_cond (ProducesBool) Cond) +(rule (bool_cond (ProducesBool.ProducesBool _ cond)) cond) + ;; Invert boolean condition. (decl invert_bool (ProducesBool) ProducesBool) (rule (invert_bool (ProducesBool.ProducesBool producer cond)) @@ -3358,11 +3352,6 @@ (rule (cond_br_bool (ProducesBool.ProducesBool producer cond) taken not_taken) (with_flags_side_effect producer (cond_br taken not_taken cond))) -;; Emit a one-way conditional branch based on a boolean condition. -(decl oneway_cond_br_bool (ProducesBool MachLabel) SideEffectNoResult) -(rule (oneway_cond_br_bool (ProducesBool.ProducesBool producer cond) dest) - (with_flags_side_effect producer (oneway_cond_br dest cond))) - ;; Emit a conditional trap based on a boolean condition. (decl trap_if_bool (ProducesBool TrapCode) SideEffectNoResult) (rule (trap_if_bool (ProducesBool.ProducesBool producer cond) trap_code) diff --git a/cranelift/codegen/src/isa/s390x/inst/emit.rs b/cranelift/codegen/src/isa/s390x/inst/emit.rs index 53d383ea535a..5e4bbb8fd356 100644 --- a/cranelift/codegen/src/isa/s390x/inst/emit.rs +++ b/cranelift/codegen/src/isa/s390x/inst/emit.rs @@ -2002,21 +2002,21 @@ impl Inst { match &inst { // Replace a CondBreak with a branch to done_label. &Inst::CondBreak { cond } => { - let inst = Inst::OneWayCondBr { - target: done_label, - cond: *cond, - }; - inst.emit_with_alloc_consumer(sink, emit_info, state); + let opcode = 0xc04; // BCRL + sink.use_label_at_offset( + sink.cur_offset(), + done_label, + LabelUse::BranchRIL, + ); + put(sink, &enc_ril_c(opcode, cond.bits(), 0)); } _ => inst.emit_with_alloc_consumer(sink, emit_info, state), }; } - let inst = Inst::OneWayCondBr { - target: loop_label, - cond, - }; - inst.emit(sink, emit_info, state); + let opcode = 0xc04; // BCRL + sink.use_label_at_offset(sink.cur_offset(), loop_label, LabelUse::BranchRIL); + put(sink, &enc_ril_c(opcode, cond.bits(), 0)); // Emit label at the end of the loop. sink.bind_label(done_label, &mut state.ctrl_plane); @@ -3314,11 +3314,6 @@ impl Inst { sink.add_uncond_branch(uncond_off, uncond_off + 6, not_taken); put(sink, &enc_ril_c(opcode, 15, 0)); } - &Inst::OneWayCondBr { target, cond } => { - let opcode = 0xc04; // BCRL - sink.use_label_at_offset(sink.cur_offset(), target, LabelUse::BranchRIL); - put(sink, &enc_ril_c(opcode, cond.bits(), 0)); - } &Inst::Nop0 => {} &Inst::Nop2 => { put(sink, &enc_e(0x0707)); @@ -3341,13 +3336,23 @@ impl Inst { put_with_trap(sink, &enc[0..4], trap_code); put(sink, &enc[4..6]); } - &Inst::JTSequence { ridx, ref targets } => { + &Inst::JTSequence { + ridx, + default, + default_cond, + ref targets, + } => { let table_label = sink.get_label(); // This sequence is *one* instruction in the vcode, and is expanded only here at // emission time, because we cannot allow the regalloc to insert spills/reloads in // the middle; we depend on hardcoded PC-rel addressing below. + // Branch to the default target if the given default condition is true. + let opcode = 0xc04; // BCRL + sink.use_label_at_offset(sink.cur_offset(), default, LabelUse::BranchRIL); + put(sink, &enc_ril_c(opcode, default_cond.bits(), 0)); + // Set temp register to address of jump table. let rtmp = writable_spilltmp_reg(); let inst = Inst::LoadAddr { diff --git a/cranelift/codegen/src/isa/s390x/inst/mod.rs b/cranelift/codegen/src/isa/s390x/inst/mod.rs index 75e7daab707f..139255c2657c 100644 --- a/cranelift/codegen/src/isa/s390x/inst/mod.rs +++ b/cranelift/codegen/src/isa/s390x/inst/mod.rs @@ -221,7 +221,6 @@ impl Inst { | Inst::Jump { .. } | Inst::CondBr { .. } | Inst::TrapIf { .. } - | Inst::OneWayCondBr { .. } | Inst::IndirectBr { .. } | Inst::Debugtrap | Inst::Trap { .. } @@ -952,7 +951,7 @@ fn s390x_get_operands(inst: &mut Inst, collector: &mut DenyReuseVisitor { collector.reg_use(rn); } - Inst::CondBr { .. } | Inst::OneWayCondBr { .. } => {} + Inst::CondBr { .. } => {} Inst::Nop0 | Inst::Nop2 => {} Inst::Debugtrap => {} Inst::Trap { .. } => {} @@ -1087,10 +1086,6 @@ impl MachInst for Inst { &Inst::ReturnCall { .. } | &Inst::ReturnCallInd { .. } => MachTerminator::RetCall, &Inst::Jump { .. } => MachTerminator::Uncond, &Inst::CondBr { .. } => MachTerminator::Cond, - &Inst::OneWayCondBr { .. } => { - // Explicitly invisible to CFG processing. - MachTerminator::None - } &Inst::IndirectBr { .. } => MachTerminator::Indirect, &Inst::JTSequence { .. } => MachTerminator::Indirect, _ => MachTerminator::None, @@ -3212,11 +3207,6 @@ impl Inst { let cond = cond.pretty_print_default(); format!("jg{cond} {taken} ; jg {not_taken}") } - &Inst::OneWayCondBr { target, cond } => { - let target = target.to_string(); - let cond = cond.pretty_print_default(); - format!("jg{cond} {target}") - } &Inst::Debugtrap => ".word 0x0001 # debugtrap".to_string(), &Inst::Trap { trap_code } => { format!(".word 0x0000 # trap={trap_code}") @@ -3225,25 +3215,34 @@ impl Inst { let cond = cond.pretty_print_default(); format!("jg{cond} .+2 # trap={trap_code}") } - &Inst::JTSequence { ridx, ref targets } => { + &Inst::JTSequence { + ridx, + default, + default_cond, + ref targets, + } => { let ridx = pretty_print_reg(ridx); let rtmp = pretty_print_reg(writable_spilltmp_reg().to_reg()); - // The first entry is the default target, which is not emitted - // into the jump table, so we skip it here. It is only in the - // list so MachTerminator will see the potential target. let jt_entries: String = targets .iter() - .skip(1) .map(|label| format!(" {}", label.to_string())) .collect(); format!( concat!( + "jg{} {} ; ", "larl {}, 14 ; ", "agf {}, 0({}, {}) ; ", "br {} ; ", "jt_entries{}" ), - rtmp, rtmp, rtmp, ridx, rtmp, jt_entries, + default_cond.pretty_print_default(), + default.to_string(), + rtmp, + rtmp, + rtmp, + ridx, + rtmp, + jt_entries, ) } &Inst::LoadSymbolReloc { diff --git a/cranelift/codegen/src/isa/s390x/lower.isle b/cranelift/codegen/src/isa/s390x/lower.isle index 3b403ef5b37b..17a9eed56623 100644 --- a/cranelift/codegen/src/isa/s390x/lower.isle +++ b/cranelift/codegen/src/isa/s390x/lower.isle @@ -3729,17 +3729,17 @@ ;; list of branch targets per index value. (rule (lower_branch (br_table val_idx _) (jump_table_targets default targets)) (let ((idx Reg (put_in_reg_zext64 val_idx)) - ;; Bounds-check the index and branch to default. - ;; This is an internal branch that is not a terminator insn. - ;; Instead, the default target is listed a potential target - ;; in the final JTSequence, which is the block terminator. + ;; Bounds-check the index and create a ProducesBool that + ;; denotes the "default target" condition. (cond ProducesBool (bool (icmpu_uimm32 $I64 idx (jump_table_size targets)) (intcc_as_cond (IntCC.UnsignedGreaterThanOrEqual)))) - (_ Unit (emit_side_effect (oneway_cond_br_bool cond default)))) + (default_cond_producer ProducesFlags (bool_producer cond)) + (default_cond Cond (bool_cond cond))) ;; Scale the index by the element size, and then emit the ;; compound instruction that does: ;; + ;; [cond branch to default] ;; larl %r1, ;; agf %r1, 0(%r1, %rScaledIndex) ;; br %r1 @@ -3751,7 +3751,10 @@ ;; PC-rel offset to the jumptable would be incorrect. ;; (The alternative is to introduce a relocation pass ;; for inlined jumptables, which is much worse, IMHO.) - (emit_side_effect (jt_sequence (lshl_imm $I64 idx 2) targets)))) + (let ((shifted_idx Reg (lshl_imm $I64 idx 2))) + (emit_side_effect (with_flags_side_effect + default_cond_producer + (jt_sequence shifted_idx default default_cond targets)))))) ;;;; Rules for `brif` ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;; diff --git a/cranelift/filetests/filetests/isa/s390x/jumptable.clif b/cranelift/filetests/filetests/isa/s390x/jumptable.clif index 2c71a5019305..32acf6ea6779 100644 --- a/cranelift/filetests/filetests/isa/s390x/jumptable.clif +++ b/cranelift/filetests/filetests/isa/s390x/jumptable.clif @@ -29,34 +29,33 @@ block5(v5: i32): ; VCode: ; block0: ; llgfr %r3, %r2 +; sllg %r4, %r3, 2 ; clgfi %r3, 3 -; jghe label4 -; sllg %r3, %r3, 2 -; larl %r1, 14 ; agf %r1, 0(%r1, %r3) ; br %r1 ; jt_entries label2 label1 +; jghe label4 ; larl %r1, 14 ; agf %r1, 0(%r1, %r4) ; br %r1 ; jt_entries label3 label2 label1 ; block1: -; lhi %r5, 3 +; lhi %r4, 3 ; jg label5 ; block2: -; lhi %r5, 2 +; lhi %r4, 2 ; jg label5 ; block3: -; lhi %r5, 1 +; lhi %r4, 1 ; jg label5 ; block4: -; lhi %r5, 4 +; lhi %r4, 4 ; jg label5 ; block5: -; ar %r2, %r5 +; ar %r2, %r4 ; br %r14 ; ; Disassembled: ; block0: ; offset 0x0 ; llgfr %r3, %r2 +; sllg %r4, %r3, 2 ; clgfi %r3, 3 ; jghe 0x4e -; sllg %r3, %r3, 2 ; larl %r1, 0x24 -; agf %r1, 0(%r3, %r1) +; agf %r1, 0(%r4, %r1) ; br %r1 ; .byte 0x00, 0x00 ; .byte 0x00, 0x20 @@ -65,17 +64,17 @@ block5(v5: i32): ; .byte 0x00, 0x00 ; .byte 0x00, 0x0c ; block1: ; offset 0x30 -; lhi %r5, 3 +; lhi %r4, 3 ; jg 0x52 ; block2: ; offset 0x3a -; lhi %r5, 2 +; lhi %r4, 2 ; jg 0x52 ; block3: ; offset 0x44 -; lhi %r5, 1 +; lhi %r4, 1 ; jg 0x52 ; block4: ; offset 0x4e -; lhi %r5, 4 +; lhi %r4, 4 ; block5: ; offset 0x52 -; ar %r2, %r5 +; ar %r2, %r4 ; br %r14