Skip to content

Commit 3ddbe97

Browse files
committed
[cc] many bug fixes, and associated unit tests
1 parent b85a67f commit 3ddbe97

File tree

9 files changed

+837
-49
lines changed

9 files changed

+837
-49
lines changed

cc/arch/regalloc.rs

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -341,7 +341,10 @@ where
341341
}
342342
})
343343
.collect();
344-
result.sort_by_key(|i| i.start);
344+
// Sort by (start, pseudo) to ensure deterministic ordering
345+
// This is critical when multiple intervals have the same start position
346+
// (e.g., all arguments start at 0), as HashMap iteration is non-deterministic
347+
result.sort_by_key(|i| (i.start, i.pseudo.0));
345348
(result, constraint_points)
346349
}
347350

cc/arch/x86_64/codegen.rs

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,14 @@ impl X86_64CodeGen {
8282
offset: -adjusted,
8383
})
8484
}
85+
Loc::IncomingArg(offset) => {
86+
// Incoming stack argument: at [rbp + offset] (positive offset)
87+
// No callee_saved_offset adjustment needed - these are above the return address
88+
GpOperand::Mem(MemAddr::BaseOffset {
89+
base: Reg::Rbp,
90+
offset: *offset,
91+
})
92+
}
8593
Loc::Imm(v) => GpOperand::Imm(*v),
8694
Loc::FImm(_, _) => GpOperand::Imm(0), // FP immediates handled separately
8795
Loc::Xmm(_) => GpOperand::Imm(0), // XMM handled separately
@@ -616,6 +624,16 @@ impl X86_64CodeGen {
616624
}),
617625
});
618626
}
627+
Loc::IncomingArg(offset) => {
628+
self.push_lir(X86Inst::Cmp {
629+
size: op_size,
630+
src: GpOperand::Imm(0),
631+
dst: GpOperand::Mem(MemAddr::BaseOffset {
632+
base: Reg::Rbp,
633+
offset: *offset,
634+
}),
635+
});
636+
}
619637
Loc::Imm(v) => {
620638
let target = if *v != 0 { insn.bb_true } else { insn.bb_false };
621639
if let Some(target) = target {
@@ -1002,6 +1020,17 @@ impl X86_64CodeGen {
10021020
dst: GpOperand::Reg(dst),
10031021
});
10041022
}
1023+
Loc::IncomingArg(offset) => {
1024+
// LIR: memory-to-register move from incoming stack arg
1025+
self.push_lir(X86Inst::Mov {
1026+
size: op_size,
1027+
src: GpOperand::Mem(MemAddr::BaseOffset {
1028+
base: Reg::Rbp,
1029+
offset,
1030+
}),
1031+
dst: GpOperand::Reg(dst),
1032+
});
1033+
}
10051034
Loc::Imm(v) => {
10061035
// x86-64: movl sign-extends to 64-bit, movq only works with 32-bit signed immediates
10071036
// For values outside 32-bit signed range, use movabsq
@@ -2274,6 +2303,9 @@ impl X86_64CodeGen {
22742303
let adjusted = offset + self.callee_saved_offset;
22752304
format!("-{}(%rbp)", adjusted)
22762305
}
2306+
Loc::IncomingArg(offset) => {
2307+
format!("{}(%rbp)", offset)
2308+
}
22772309
Loc::Imm(v) => format!("${}", v),
22782310
Loc::Xmm(xmm) => xmm.name().to_string(),
22792311
Loc::FImm(_, _) => {

cc/arch/x86_64/float.rs

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -730,6 +730,16 @@ impl X86_64CodeGen {
730730
dst: XmmOperand::Reg(dst),
731731
});
732732
}
733+
Loc::IncomingArg(offset) => {
734+
self.push_lir(X86Inst::MovFp {
735+
size: fp_size,
736+
src: XmmOperand::Mem(MemAddr::BaseOffset {
737+
base: Reg::Rbp,
738+
offset,
739+
}),
740+
dst: XmmOperand::Reg(dst),
741+
});
742+
}
733743
Loc::FImm(v, imm_size) => {
734744
// Use the size from the FImm, not the passed-in size
735745
// This ensures float constants are loaded as float, not double

cc/arch/x86_64/regalloc.rs

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -404,6 +404,9 @@ pub enum Loc {
404404
Xmm(XmmReg),
405405
/// On the stack at [rbp - offset]
406406
Stack(i32),
407+
/// Incoming stack argument at [rbp + offset] (positive offset, above return address)
408+
/// Used for function parameters 7+ that are passed on the stack by the caller
409+
IncomingArg(i32),
407410
/// Immediate integer constant
408411
Imm(i64),
409412
/// Immediate float constant (value, size in bits)
@@ -527,9 +530,11 @@ impl RegAlloc {
527530
self.free_xmm_regs.retain(|&r| r != fp_arg_regs[fp_arg_idx]);
528531
self.fp_pseudos.insert(pseudo.id);
529532
} else {
533+
// Stack-passed FP argument: at [rbp + offset]
534+
// 16 = saved rbp (8) + return address (8)
530535
let offset =
531536
16 + (i - int_arg_regs.len() - fp_arg_regs.len()) as i32 * 8;
532-
self.locations.insert(pseudo.id, Loc::Stack(-offset));
537+
self.locations.insert(pseudo.id, Loc::IncomingArg(offset));
533538
}
534539
fp_arg_idx += 1;
535540
} else {
@@ -538,8 +543,10 @@ impl RegAlloc {
538543
.insert(pseudo.id, Loc::Reg(int_arg_regs[int_arg_idx]));
539544
self.free_regs.retain(|&r| r != int_arg_regs[int_arg_idx]);
540545
} else {
541-
let offset = 16 + (i - int_arg_regs.len()) as i32 * 8;
542-
self.locations.insert(pseudo.id, Loc::Stack(-offset));
546+
// Stack-passed integer argument: at [rbp + offset]
547+
// 16 = saved rbp (8) + return address (8)
548+
let offset = 16 + (int_arg_idx - int_arg_regs.len()) as i32 * 8;
549+
self.locations.insert(pseudo.id, Loc::IncomingArg(offset));
543550
}
544551
int_arg_idx += 1;
545552
}

cc/ir/dce.rs

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,11 @@ fn get_uses(insn: &Instruction) -> Vec<PseudoId> {
8686
}
8787
}
8888

89+
// Indirect call target (function pointer)
90+
if let Some(indirect) = insn.indirect_target {
91+
uses.push(indirect);
92+
}
93+
8994
uses
9095
}
9196

@@ -507,4 +512,74 @@ mod tests {
507512

508513
assert_eq!(func.blocks[0].insns[1].op, Opcode::Store);
509514
}
515+
516+
#[test]
517+
fn test_indirect_call_target_is_use() {
518+
// Test that get_uses() includes indirect_target for function pointer calls.
519+
// This prevents DCE from eliminating the instruction that computes
520+
// the function pointer before an indirect call.
521+
let types = TypeTable::new(64);
522+
let call_insn = Instruction::call_indirect(
523+
Some(PseudoId(0)), // return value target
524+
PseudoId(5), // func_addr (the function pointer)
525+
vec![PseudoId(1), PseudoId(2)], // args
526+
vec![types.int_id, types.int_id], // arg_types
527+
types.int_id,
528+
32,
529+
);
530+
531+
let uses = get_uses(&call_insn);
532+
533+
// Verify indirect_target is in the uses list
534+
assert!(
535+
uses.contains(&PseudoId(5)),
536+
"get_uses should include indirect_target"
537+
);
538+
// Also verify call arguments are in uses
539+
assert!(uses.contains(&PseudoId(1)));
540+
assert!(uses.contains(&PseudoId(2)));
541+
}
542+
543+
#[test]
544+
fn test_indirect_call_keeps_func_ptr_live() {
545+
// Full DCE test: verify an indirect call keeps its function pointer live.
546+
let types = TypeTable::new(64);
547+
let mut func = Function::new("test", types.int_id);
548+
549+
func.add_pseudo(Pseudo::reg(PseudoId(0), 0)); // result
550+
func.add_pseudo(Pseudo::reg(PseudoId(1), 1)); // func pointer
551+
func.add_pseudo(Pseudo::val(PseudoId(2), 42)); // arg
552+
553+
let mut bb = BasicBlock::new(BasicBlockId(0));
554+
bb.add_insn(Instruction::new(Opcode::Entry));
555+
556+
// %1 = load the function pointer (this should stay live)
557+
let mut load = Instruction::new(Opcode::Load);
558+
load.target = Some(PseudoId(1));
559+
load.src = vec![PseudoId(2)]; // load from some address
560+
load.typ = Some(types.pointer_to(types.int_id));
561+
load.size = 64;
562+
bb.add_insn(load);
563+
564+
// %0 = call_indirect %1(%2)
565+
let call = Instruction::call_indirect(
566+
Some(PseudoId(0)),
567+
PseudoId(1), // indirect through %1
568+
vec![PseudoId(2)], // args
569+
vec![types.int_id], // arg_types
570+
types.int_id,
571+
32,
572+
);
573+
bb.add_insn(call);
574+
575+
bb.add_insn(Instruction::ret(Some(PseudoId(0))));
576+
func.add_block(bb);
577+
func.entry = BasicBlockId(0);
578+
579+
let changed = run(&mut func);
580+
581+
// The load instruction that defines %1 (func pointer) should NOT be
582+
// eliminated because %1 is used as indirect_target in the call
583+
assert!(!changed || func.blocks[0].insns[1].op == Opcode::Load);
584+
}
510585
}

0 commit comments

Comments
 (0)