Skip to content

Commit 54cfa4d

Browse files
elliotttelliottt
authored
cranelift: Fix implicit pointer argument register use (#5301)
* Fix arg handling to write to VRegs instead of physical regs * Make is_included_in_clobbers required, and handle Args on x64 and riscv64
1 parent 7a31c5b commit 54cfa4d

File tree

10 files changed

+51
-28
lines changed

10 files changed

+51
-28
lines changed

‎cranelift/codegen/src/isa/aarch64/inst/mod.rs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1142,6 +1142,7 @@ impl MachInst for Inst {
11421142
// See the note in [crate::isa::aarch64::abi::is_caller_save_reg] for
11431143
// more information on this ABI-implementation hack.
11441144
match self {
1145+
&Inst::Args { .. } => false,
11451146
&Inst::Call { ref info } => info.caller_callconv != info.callee_callconv,
11461147
&Inst::CallInd { ref info } => info.caller_callconv != info.callee_callconv,
11471148
_ => true,

‎cranelift/codegen/src/isa/riscv64/inst/mod.rs‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -657,7 +657,10 @@ impl MachInst for Inst {
657657
}
658658

659659
fn is_included_in_clobbers(&self) -> bool {
660-
true
660+
match self {
661+
&Inst::Args { .. } => false,
662+
_ => true,
663+
}
661664
}
662665

663666
fn is_args(&self) -> bool {

‎cranelift/codegen/src/isa/s390x/inst/mod.rs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1109,6 +1109,7 @@ impl MachInst for Inst {
11091109
// half-caller-save, half-callee-save SysV ABI for some vector
11101110
// registers.
11111111
match self {
1112+
&Inst::Args { .. } => false,
11121113
&Inst::Call { ref info, .. } => info.caller_callconv != info.callee_callconv,
11131114
&Inst::CallInd { ref info, .. } => info.caller_callconv != info.callee_callconv,
11141115
_ => true,

‎cranelift/codegen/src/isa/x64/inst/mod.rs‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2236,6 +2236,13 @@ impl MachInst for Inst {
22362236
}
22372237
}
22382238

2239+
fn is_included_in_clobbers(&self) -> bool {
2240+
match self {
2241+
&Inst::Args { .. } => false,
2242+
_ => true,
2243+
}
2244+
}
2245+
22392246
fn is_args(&self) -> bool {
22402247
match self {
22412248
Self::Args { .. } => true,

‎cranelift/codegen/src/machinst/abi.rs‎

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1396,6 +1396,7 @@ impl<M: ABIMachineSpec> Callee<M> {
13961396
sigs: &SigSet,
13971397
idx: usize,
13981398
into_regs: ValueRegs<Writable<Reg>>,
1399+
vregs: &mut VRegAllocator<M::I>,
13991400
) -> SmallInstVec<M::I> {
14001401
let mut insts = smallvec![];
14011402
let mut copy_arg_slot_to_reg = |slot: &ABIArgSlot, into_reg: &Writable<Reg>| {
@@ -1470,7 +1471,14 @@ impl<M: ABIMachineSpec> Callee<M> {
14701471
let into_reg = into_regs.only_reg().unwrap();
14711472
// We need to dereference the pointer.
14721473
let base = match &pointer {
1473-
&ABIArgSlot::Reg { reg, .. } => Reg::from(reg),
1474+
&ABIArgSlot::Reg { reg, ty, .. } => {
1475+
let tmp = vregs.alloc(ty).unwrap().only_reg().unwrap();
1476+
self.reg_args.push(ArgPair {
1477+
vreg: Writable::from_reg(tmp),
1478+
preg: reg.into(),
1479+
});
1480+
tmp
1481+
}
14741482
&ABIArgSlot::Stack { offset, ty, .. } => {
14751483
// In this case we need a temp register to hold the address.
14761484
// This was allocated in the `init` routine.
@@ -1606,12 +1614,17 @@ impl<M: ABIMachineSpec> Callee<M> {
16061614
/// values or an otherwise large return value that must be passed on the
16071615
/// stack; typically the ABI specifies an extra hidden argument that is a
16081616
/// pointer to that memory.
1609-
pub fn gen_retval_area_setup(&mut self, sigs: &SigSet) -> Option<M::I> {
1617+
pub fn gen_retval_area_setup(
1618+
&mut self,
1619+
sigs: &SigSet,
1620+
vregs: &mut VRegAllocator<M::I>,
1621+
) -> Option<M::I> {
16101622
if let Some(i) = sigs[self.sig].stack_ret_arg {
16111623
let insts = self.gen_copy_arg_to_regs(
16121624
sigs,
16131625
i.into(),
16141626
ValueRegs::one(self.ret_area_ptr.unwrap()),
1627+
vregs,
16151628
);
16161629
insts.into_iter().next().map(|inst| {
16171630
trace!(

‎cranelift/codegen/src/machinst/lower.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -573,7 +573,7 @@ impl<'func, I: VCodeInst> Lower<'func, I> {
573573
.vcode
574574
.vcode
575575
.abi
576-
.gen_copy_arg_to_regs(&self.vcode.vcode.sigs, i, regs)
576+
.gen_copy_arg_to_regs(&self.vcode.vcode.sigs, i, regs, &mut self.vregs)
577577
.into_iter()
578578
{
579579
self.emit(insn);
@@ -601,7 +601,7 @@ impl<'func, I: VCodeInst> Lower<'func, I> {
601601
.vcode
602602
.vcode
603603
.abi
604-
.gen_retval_area_setup(&self.vcode.vcode.sigs)
604+
.gen_retval_area_setup(&self.vcode.vcode.sigs, &mut self.vregs)
605605
{
606606
self.emit(insn);
607607
}

‎cranelift/codegen/src/machinst/mod.rs‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -104,9 +104,7 @@ pub trait MachInst: Clone + Debug {
104104
fn is_args(&self) -> bool;
105105

106106
/// Should this instruction be included in the clobber-set?
107-
fn is_included_in_clobbers(&self) -> bool {
108-
true
109-
}
107+
fn is_included_in_clobbers(&self) -> bool;
110108

111109
/// Generate a move.
112110
fn gen_move(to_reg: Writable<Reg>, from_reg: Reg, ty: Type) -> Self;

‎cranelift/filetests/filetests/isa/s390x/arithmetic.clif‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -703,7 +703,7 @@ block0(v0: i128, v1: i128):
703703

704704
; stmg %r7, %r15, 56(%r15)
705705
; block0:
706-
; lgr %r14, %r2
706+
; lgr %r10, %r2
707707
; vl %v0, 0(%r3)
708708
; vl %v1, 0(%r4)
709709
; lgdr %r4, %f0
@@ -718,7 +718,7 @@ block0(v0: i128, v1: i128):
718718
; agrk %r4, %r2, %r8
719719
; agr %r5, %r4
720720
; vlvgp %v5, %r5, %r3
721-
; lgr %r2, %r14
721+
; lgr %r2, %r10
722722
; vst %v5, 0(%r2)
723723
; lmg %r7, %r15, 56(%r15)
724724
; br %r14

‎cranelift/filetests/filetests/isa/s390x/conversions.clif‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -348,8 +348,8 @@ block0(v0: i128):
348348

349349
; block0:
350350
; vl %v0, 0(%r2)
351-
; vgbm %v2, 0
352-
; vceqgs %v4, %v0, %v2
351+
; vgbm %v3, 0
352+
; vceqgs %v5, %v0, %v3
353353
; lghi %r2, 0
354354
; locghine %r2, -1
355355
; br %r14
@@ -362,8 +362,8 @@ block0(v0: i128):
362362

363363
; block0:
364364
; vl %v0, 0(%r2)
365-
; vgbm %v2, 0
366-
; vceqgs %v4, %v0, %v2
365+
; vgbm %v3, 0
366+
; vceqgs %v5, %v0, %v3
367367
; lhi %r2, 0
368368
; lochine %r2, -1
369369
; br %r14
@@ -376,8 +376,8 @@ block0(v0: i128):
376376

377377
; block0:
378378
; vl %v0, 0(%r2)
379-
; vgbm %v2, 0
380-
; vceqgs %v4, %v0, %v2
379+
; vgbm %v3, 0
380+
; vceqgs %v5, %v0, %v3
381381
; lhi %r2, 0
382382
; lochine %r2, -1
383383
; br %r14
@@ -390,8 +390,8 @@ block0(v0: i128):
390390

391391
; block0:
392392
; vl %v0, 0(%r2)
393-
; vgbm %v2, 0
394-
; vceqgs %v4, %v0, %v2
393+
; vgbm %v3, 0
394+
; vceqgs %v5, %v0, %v3
395395
; lhi %r2, 0
396396
; lochine %r2, -1
397397
; br %r14

‎cranelift/filetests/filetests/isa/s390x/icmp-i128.clif‎

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ block0(v0: i128, v1: i128):
1010
; block0:
1111
; vl %v0, 0(%r2)
1212
; vl %v1, 0(%r3)
13-
; vceqgs %v4, %v0, %v1
13+
; vceqgs %v5, %v0, %v1
1414
; lhi %r2, 0
1515
; lochie %r2, 1
1616
; br %r14
@@ -24,7 +24,7 @@ block0(v0: i128, v1: i128):
2424
; block0:
2525
; vl %v0, 0(%r2)
2626
; vl %v1, 0(%r3)
27-
; vceqgs %v4, %v0, %v1
27+
; vceqgs %v5, %v0, %v1
2828
; lhi %r2, 0
2929
; lochine %r2, 1
3030
; br %r14
@@ -38,7 +38,7 @@ block0(v0: i128, v1: i128):
3838
; block0:
3939
; vl %v0, 0(%r2)
4040
; vl %v1, 0(%r3)
41-
; vecg %v0, %v1 ; jne 10 ; vchlgs %v4, %v1, %v0
41+
; vecg %v0, %v1 ; jne 10 ; vchlgs %v5, %v1, %v0
4242
; lhi %r2, 0
4343
; lochil %r2, 1
4444
; br %r14
@@ -52,7 +52,7 @@ block0(v0: i128, v1: i128):
5252
; block0:
5353
; vl %v0, 0(%r2)
5454
; vl %v1, 0(%r3)
55-
; vecg %v1, %v0 ; jne 10 ; vchlgs %v4, %v0, %v1
55+
; vecg %v1, %v0 ; jne 10 ; vchlgs %v5, %v0, %v1
5656
; lhi %r2, 0
5757
; lochil %r2, 1
5858
; br %r14
@@ -66,7 +66,7 @@ block0(v0: i128, v1: i128):
6666
; block0:
6767
; vl %v0, 0(%r2)
6868
; vl %v1, 0(%r3)
69-
; vecg %v1, %v0 ; jne 10 ; vchlgs %v4, %v0, %v1
69+
; vecg %v1, %v0 ; jne 10 ; vchlgs %v5, %v0, %v1
7070
; lhi %r2, 0
7171
; lochinl %r2, 1
7272
; br %r14
@@ -80,7 +80,7 @@ block0(v0: i128, v1: i128):
8080
; block0:
8181
; vl %v0, 0(%r2)
8282
; vl %v1, 0(%r3)
83-
; vecg %v0, %v1 ; jne 10 ; vchlgs %v4, %v1, %v0
83+
; vecg %v0, %v1 ; jne 10 ; vchlgs %v5, %v1, %v0
8484
; lhi %r2, 0
8585
; lochinl %r2, 1
8686
; br %r14
@@ -94,7 +94,7 @@ block0(v0: i128, v1: i128):
9494
; block0:
9595
; vl %v0, 0(%r2)
9696
; vl %v1, 0(%r3)
97-
; veclg %v0, %v1 ; jne 10 ; vchlgs %v4, %v1, %v0
97+
; veclg %v0, %v1 ; jne 10 ; vchlgs %v5, %v1, %v0
9898
; lhi %r2, 0
9999
; lochil %r2, 1
100100
; br %r14
@@ -108,7 +108,7 @@ block0(v0: i128, v1: i128):
108108
; block0:
109109
; vl %v0, 0(%r2)
110110
; vl %v1, 0(%r3)
111-
; veclg %v1, %v0 ; jne 10 ; vchlgs %v4, %v0, %v1
111+
; veclg %v1, %v0 ; jne 10 ; vchlgs %v5, %v0, %v1
112112
; lhi %r2, 0
113113
; lochil %r2, 1
114114
; br %r14
@@ -122,7 +122,7 @@ block0(v0: i128, v1: i128):
122122
; block0:
123123
; vl %v0, 0(%r2)
124124
; vl %v1, 0(%r3)
125-
; veclg %v1, %v0 ; jne 10 ; vchlgs %v4, %v0, %v1
125+
; veclg %v1, %v0 ; jne 10 ; vchlgs %v5, %v0, %v1
126126
; lhi %r2, 0
127127
; lochinl %r2, 1
128128
; br %r14
@@ -136,7 +136,7 @@ block0(v0: i128, v1: i128):
136136
; block0:
137137
; vl %v0, 0(%r2)
138138
; vl %v1, 0(%r3)
139-
; veclg %v0, %v1 ; jne 10 ; vchlgs %v4, %v1, %v0
139+
; veclg %v0, %v1 ; jne 10 ; vchlgs %v5, %v1, %v0
140140
; lhi %r2, 0
141141
; lochinl %r2, 1
142142
; br %r14

0 commit comments

Comments
 (0)