Skip to content

Commit 8cf33a5

Browse files
jgarzikclaude
andcommitted
[cc] Fix loop regalloc, macOS .comm alignment, and array alignment
- Loop-carried values now use callee-saved registers to prevent clobber across iterations when no function calls force spills. Added `in_loop` flag to LiveInterval for intervals spanning loop back edges. - macOS .comm directive now emits log2(alignment) per Apple assembler convention, while Linux/FreeBSD continue using byte alignment. - Global arrays >= 16 bytes now get 16-byte alignment, matching clang's LargeArrayMinWidth optimization for vectorization. These fixes enable successful compilation of zlib (`make test` passes). 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
1 parent 5a99d3e commit 8cf33a5

File tree

7 files changed

+485
-9
lines changed

7 files changed

+485
-9
lines changed

cc/arch/codegen.rs

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -210,7 +210,11 @@ impl<I: LirInst + EmitAsm> CodeGenBase<I> {
210210
let is_static = types.get(*typ).modifiers.contains(TypeModifiers::STATIC);
211211

212212
// Get alignment from type info - use natural alignment per ABI
213-
let align = types.alignment(*typ) as u32;
213+
let mut align = types.alignment(*typ) as u32;
214+
// Use 16-byte alignment for arrays >= 16 bytes (matches clang behavior for optimization)
215+
if matches!(types.get(*typ).kind, crate::types::TypeKind::Array) && size >= 16 {
216+
align = align.max(16);
217+
}
214218

215219
// Use .comm for uninitialized external (non-static) globals
216220
let use_bss = matches!(init, Initializer::None) && !is_static;

cc/arch/lir.rs

Lines changed: 59 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -617,12 +617,24 @@ impl EmitAsm for Directive {
617617
}
618618
}
619619
Directive::Comm { sym, size, align } => {
620+
// macOS uses log2(alignment) for .comm, Linux uses byte alignment
621+
let align_value = match target.os {
622+
Os::MacOS => {
623+
// Convert byte alignment to log2
624+
if *align == 0 {
625+
0
626+
} else {
627+
align.trailing_zeros()
628+
}
629+
}
630+
Os::Linux | Os::FreeBSD => *align,
631+
};
620632
let _ = writeln!(
621633
out,
622634
".comm {},{},{}",
623635
sym.format_for_target(target),
624636
size,
625-
align
637+
align_value
626638
);
627639
}
628640

@@ -890,13 +902,58 @@ mod tests {
890902
fn test_directive_comm() {
891903
let linux = Target::new(Arch::X86_64, Os::Linux);
892904
let macos = Target::new(Arch::X86_64, Os::MacOS);
905+
let freebsd = Target::new(Arch::X86_64, Os::FreeBSD);
893906

907+
// Linux uses byte alignment directly
894908
let mut out = String::new();
895909
Directive::comm("my_var", 8, 8).emit(&linux, &mut out);
896910
assert_eq!(out, ".comm my_var,8,8\n");
897911

912+
// macOS uses log2(alignment): 8 bytes = 2^3, so log2(8) = 3
898913
let mut out = String::new();
899914
Directive::comm("my_var", 8, 8).emit(&macos, &mut out);
900-
assert_eq!(out, ".comm _my_var,8,8\n");
915+
assert_eq!(out, ".comm _my_var,8,3\n");
916+
917+
// FreeBSD uses byte alignment like Linux
918+
let mut out = String::new();
919+
Directive::comm("my_var", 8, 8).emit(&freebsd, &mut out);
920+
assert_eq!(out, ".comm my_var,8,8\n");
921+
922+
// Test alignment=1: macOS log2(1)=0, Linux/FreeBSD=1
923+
let mut out = String::new();
924+
Directive::comm("byte_var", 1, 1).emit(&linux, &mut out);
925+
assert_eq!(out, ".comm byte_var,1,1\n");
926+
927+
let mut out = String::new();
928+
Directive::comm("byte_var", 1, 1).emit(&macos, &mut out);
929+
assert_eq!(out, ".comm _byte_var,1,0\n");
930+
931+
// Test alignment=2: macOS log2(2)=1, Linux=2
932+
let mut out = String::new();
933+
Directive::comm("short_var", 2, 2).emit(&linux, &mut out);
934+
assert_eq!(out, ".comm short_var,2,2\n");
935+
936+
let mut out = String::new();
937+
Directive::comm("short_var", 2, 2).emit(&macos, &mut out);
938+
assert_eq!(out, ".comm _short_var,2,1\n");
939+
940+
// Test alignment=4: macOS log2(4)=2, Linux=4
941+
let mut out = String::new();
942+
Directive::comm("int_var", 4, 4).emit(&linux, &mut out);
943+
assert_eq!(out, ".comm int_var,4,4\n");
944+
945+
let mut out = String::new();
946+
Directive::comm("int_var", 4, 4).emit(&macos, &mut out);
947+
assert_eq!(out, ".comm _int_var,4,2\n");
948+
949+
// Test alignment=16: macOS log2(16)=4, Linux=16
950+
// (used for large arrays per clang's LargeArrayMinWidth)
951+
let mut out = String::new();
952+
Directive::comm("big_array", 64, 16).emit(&linux, &mut out);
953+
assert_eq!(out, ".comm big_array,64,16\n");
954+
955+
let mut out = String::new();
956+
Directive::comm("big_array", 64, 16).emit(&macos, &mut out);
957+
assert_eq!(out, ".comm _big_array,64,4\n");
901958
}
902959
}

cc/arch/regalloc.rs

Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,8 @@ pub struct LiveInterval {
2323
pub pseudo: PseudoId,
2424
pub start: usize,
2525
pub end: usize,
26+
/// True if this interval spans a loop back edge (used within a loop)
27+
pub in_loop: bool,
2628
}
2729

2830
/// A point in the function where register constraints apply.
@@ -143,6 +145,7 @@ where
143145
first_def: usize,
144146
last_def: usize,
145147
last_use: usize,
148+
in_loop: bool,
146149
}
147150

148151
let mut intervals: HashMap<PseudoId, IntervalInfo> = HashMap::new();
@@ -170,6 +173,7 @@ where
170173
first_def: 0,
171174
last_def: 0,
172175
last_use: 0,
176+
in_loop: false,
173177
},
174178
);
175179
}
@@ -193,6 +197,7 @@ where
193197
first_def: pos,
194198
last_def: pos,
195199
last_use: pos,
200+
in_loop: false,
196201
});
197202
}
198203

@@ -207,6 +212,7 @@ where
207212
first_def: pos,
208213
last_def: pos,
209214
last_use: pos,
215+
in_loop: false,
210216
},
211217
);
212218
}
@@ -224,6 +230,7 @@ where
224230
first_def: pos,
225231
last_def: pos,
226232
last_use: pos,
233+
in_loop: false,
227234
},
228235
);
229236
}
@@ -262,6 +269,7 @@ where
262269
first_def: end_pos,
263270
last_def: end_pos,
264271
last_use: end_pos,
272+
in_loop: false,
265273
},
266274
);
267275
}
@@ -281,6 +289,7 @@ where
281289
first_def: end_pos,
282290
last_def: end_pos,
283291
last_use: end_pos,
292+
in_loop: false,
284293
},
285294
);
286295
}
@@ -310,16 +319,22 @@ where
310319
}
311320
}
312321

313-
// Extend lifetimes for loop variables
322+
// Extend lifetimes for loop variables and mark them as in_loop
323+
// Only mark variables that need to persist across loop iterations:
324+
// - Defined before the loop but used within the loop
325+
// - Used at or after the back edge (survive iteration boundary)
314326
for (_from_bb, to_bb, back_edge_pos) in &loop_back_edges {
315327
let loop_start = block_start_pos.get(to_bb).copied().unwrap_or(0);
316328

317329
for info in intervals.values_mut() {
318-
if info.first_def < loop_start
330+
// This is a loop-carried value that spans iterations
331+
let spans_loop_iteration = info.first_def < loop_start
319332
&& info.last_use >= loop_start
320-
&& info.last_use <= *back_edge_pos
321-
{
333+
&& info.last_use <= *back_edge_pos;
334+
335+
if spans_loop_iteration {
322336
info.last_use = info.last_use.max(*back_edge_pos);
337+
info.in_loop = true;
323338
}
324339
}
325340
}
@@ -338,6 +353,7 @@ where
338353
pseudo: info.pseudo,
339354
start: info.first_def,
340355
end,
356+
in_loop: info.in_loop,
341357
}
342358
})
343359
.collect();

cc/arch/x86_64/regalloc.rs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -740,8 +740,9 @@ impl RegAlloc {
740740
self.locations
741741
.insert(interval.pseudo, Loc::Stack(self.stack_offset));
742742
}
743-
} else if crosses_call {
744-
// Interval crosses a call - must use callee-saved register or spill
743+
} else if crosses_call || interval.in_loop {
744+
// Interval crosses a call or is inside a loop - must use callee-saved register or spill
745+
// For loops without calls, this prevents values from being clobbered by register pressure
745746
// Also exclude registers that conflict with constraints
746747
if let Some(idx) = self
747748
.free_regs

0 commit comments

Comments
 (0)