Skip to content

Commit 2b0fb65

Browse files
authored
Merge pull request #499 from rustcoreutils/updates
[cc] Bug fixes, shared lib support
2 parents 5a99d3e + b4456fe commit 2b0fb65

File tree

14 files changed

+1585
-46
lines changed

14 files changed

+1585
-46
lines changed

cc/arch/aarch64/codegen.rs

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,8 @@ pub struct Aarch64CodeGen {
5151
pub(super) num_fixed_gp_params: usize,
5252
/// External symbols (need GOT access on macOS)
5353
pub(super) extern_symbols: HashSet<String>,
54+
/// Position-independent code mode (for shared libraries)
55+
pic_mode: bool,
5456
}
5557

5658
/// Result of computing a memory address for load/store operations
@@ -75,13 +77,18 @@ impl Aarch64CodeGen {
7577
reg_save_area_size: 0,
7678
num_fixed_gp_params: 0,
7779
extern_symbols: HashSet::new(),
80+
pic_mode: false,
7881
}
7982
}
8083

81-
/// Check if a symbol needs GOT access (extern on macOS)
84+
/// Check if a symbol needs GOT access
85+
/// - In PIC mode: all external symbols need GOT access
86+
/// - On macOS: external symbols always need GOT access (even without PIC)
8287
#[inline]
8388
pub(super) fn needs_got_access(&self, name: &str) -> bool {
84-
self.base.target.os == Os::MacOS && self.extern_symbols.contains(name)
89+
let is_extern = self.extern_symbols.contains(name);
90+
// PIC mode or macOS: external symbols need GOT
91+
(self.pic_mode || self.base.target.os == Os::MacOS) && is_extern
8592
}
8693

8794
/// Compute the actual FP-relative offset for a stack location.
@@ -2355,4 +2362,8 @@ impl CodeGenerator for Aarch64CodeGen {
23552362
fn set_emit_unwind_tables(&mut self, emit: bool) {
23562363
self.base.emit_unwind_tables = emit;
23572364
}
2365+
2366+
fn set_pic_mode(&mut self, pic: bool) {
2367+
self.pic_mode = pic;
2368+
}
23582369
}

cc/arch/codegen.rs

Lines changed: 10 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;
@@ -602,12 +606,16 @@ pub trait CodeGenerator {
602606

603607
/// Set whether to emit basic unwind tables (cfi_startproc/cfi_endproc)
604608
fn set_emit_unwind_tables(&mut self, emit: bool);
609+
610+
/// Set position-independent code mode (for shared libraries)
611+
fn set_pic_mode(&mut self, pic: bool);
605612
}
606613

607614
/// Create a code generator for the given target with options
608615
pub fn create_codegen_with_options(
609616
target: Target,
610617
emit_unwind_tables: bool,
618+
pic_mode: bool,
611619
) -> Box<dyn CodeGenerator> {
612620
use crate::target::Arch;
613621

@@ -616,5 +624,6 @@ pub fn create_codegen_with_options(
616624
Arch::Aarch64 => Box::new(super::aarch64::codegen::Aarch64CodeGen::new(target)),
617625
};
618626
codegen.set_emit_unwind_tables(emit_unwind_tables);
627+
codegen.set_pic_mode(pic_mode);
619628
codegen
620629
}

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/codegen.rs

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,8 @@ pub struct X86_64CodeGen {
4747
pub(super) unique_label_counter: u32,
4848
/// External symbols (need GOT access on macOS)
4949
pub(super) extern_symbols: HashSet<String>,
50+
/// Position-independent code mode (for shared libraries)
51+
pic_mode: bool,
5052
}
5153

5254
impl X86_64CodeGen {
@@ -62,6 +64,7 @@ impl X86_64CodeGen {
6264
num_fixed_fp_params: 0,
6365
unique_label_counter: 0,
6466
extern_symbols: HashSet::new(),
67+
pic_mode: false,
6568
}
6669
}
6770

@@ -104,10 +107,14 @@ impl X86_64CodeGen {
104107
}
105108
}
106109

107-
/// Check if a symbol needs GOT access (extern on macOS)
110+
/// Check if a symbol needs GOT access
111+
/// - In PIC mode: all external symbols need GOT access
112+
/// - On macOS: external symbols always need GOT access (even without PIC)
108113
#[inline]
109114
pub(super) fn needs_got_access(&self, name: &str) -> bool {
110-
self.base.target.os == Os::MacOS && self.extern_symbols.contains(name)
115+
let is_extern = self.extern_symbols.contains(name);
116+
// PIC mode or macOS: external symbols need GOT
117+
(self.pic_mode || self.base.target.os == Os::MacOS) && is_extern
111118
}
112119

113120
/// Emit .loc directive for source line tracking (delegates to base)
@@ -2667,4 +2674,8 @@ impl CodeGenerator for X86_64CodeGen {
26672674
fn set_emit_unwind_tables(&mut self, emit: bool) {
26682675
self.base.emit_unwind_tables = emit;
26692676
}
2677+
2678+
fn set_pic_mode(&mut self, pic: bool) {
2679+
self.pic_mode = pic;
2680+
}
26702681
}

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)