Merge pull request #498 from rustcoreutils/updates

[cc] Fix alignment, regalloc, and SSA lowering bugs

Author: jgarzik

cc/arch/codegen.rs

@@ -209,7 +209,7 @@ impl<I: LirInst + EmitAsm> CodeGenBase<I> {
         // Check storage class - skip .globl for static
         let is_static = types.get(*typ).modifiers.contains(TypeModifiers::STATIC);
 
-        // Get alignment from type info
+        // Get alignment from type info - use natural alignment per ABI
         let align = types.alignment(*typ) as u32;
 
         // Use .comm for uninitialized external (non-static) globals

cc/arch/x86_64/regalloc.rs

@@ -479,6 +479,7 @@ impl RegAlloc {
         let call_positions = find_call_positions(func);
 
         self.spill_args_across_calls(func, &intervals, &call_positions);
+        self.spill_args_across_constraints(func, &intervals, &constraint_points);
         self.allocate_alloca_to_stack(func);
         self.run_linear_scan(func, types, intervals, &call_positions, &constraint_points);
 
@@ -603,6 +604,53 @@ impl RegAlloc {
         &self.spilled_args
     }
 
+    /// Spill arguments in registers that would be clobbered by constraint points (e.g., shifts)
+    ///
+    /// For example, if the 4th parameter is in Rcx and the function contains variable shifts,
+    /// Rcx will be clobbered when the shift count is loaded. We must spill such arguments
+    /// to the stack before they get clobbered.
+    fn spill_args_across_constraints(
+        &mut self,
+        _func: &Function,
+        intervals: &[LiveInterval],
+        constraint_points: &[ConstraintPoint<Reg>],
+    ) {
+        // For each argument in a register, check if its interval is live across
+        // any constraint point that clobbers that register
+        let int_arg_regs_set: Vec<Reg> = Reg::arg_regs().to_vec();
+        for interval in intervals {
+            if let Some(Loc::Reg(reg)) = self.locations.get(&interval.pseudo) {
+                if int_arg_regs_set.contains(reg) {
+                    // Check if this register is clobbered by any constraint point
+                    // while the interval is live (and the pseudo is not involved)
+                    let needs_spill = constraint_points.iter().any(|cp| {
+                        interval.start <= cp.position
+                            && cp.position <= interval.end
+                            && !cp.involved_pseudos.contains(&interval.pseudo)
+                            && cp.clobbers.contains(reg)
+                    });
+
+                    if needs_spill {
+                        let from_reg = *reg;
+                        self.stack_offset += 8;
+                        let to_stack_offset = self.stack_offset;
+
+                        // Record the spill for codegen to emit stores in prologue
+                        self.spilled_args.push(SpilledArg {
+                            pseudo: interval.pseudo,
+                            from_reg,
+                            to_stack_offset,
+                        });
+
+                        self.locations
+                            .insert(interval.pseudo, Loc::Stack(to_stack_offset));
+                        self.free_regs.push(from_reg);
+                    }
+                }
+            }
+        }
+    }
+
     /// Force alloca results to stack to avoid clobbering issues
     fn allocate_alloca_to_stack(&mut self, func: &Function) {
         for block in &func.blocks {

cc/ir/dce.rs

@@ -294,22 +294,25 @@ fn remove_unreachable_blocks(func: &mut Function) -> bool {
     let reachable = compute_reachable(func);
     let before = func.blocks.len();
 
+    // Compute unreachable set BEFORE removing blocks
+    let all_block_ids: HashSet<_> = func.blocks.iter().map(|bb| bb.id).collect();
+    let unreachable: HashSet<_> = all_block_ids.difference(&reachable).copied().collect();
+
     // Remove unreachable blocks
     func.blocks.retain(|bb| reachable.contains(&bb.id));
 
     // Update parent/child references to remove dead blocks
-    let unreachable: HashSet<_> = func
-        .blocks
-        .iter()
-        .map(|bb| bb.id)
-        .collect::<HashSet<_>>()
-        .difference(&reachable)
-        .copied()
-        .collect();
-
     for bb in &mut func.blocks {
         bb.parents.retain(|p| !unreachable.contains(p));
         bb.children.retain(|c| !unreachable.contains(c));
+
+        // Also clean phi_list entries that reference removed blocks
+        for insn in &mut bb.insns {
+            if insn.op == Opcode::Phi {
+                insn.phi_list
+                    .retain(|(pred, _)| !unreachable.contains(pred));
+            }
+        }
     }
 
     func.blocks.len() < before

cc/ir/linearize.rs

@@ -1836,9 +1836,14 @@ impl<'a> Linearizer<'a> {
         // Condition block
         self.switch_bb(cond_bb);
         let cond_val = self.linearize_expr(cond);
-        self.emit(Instruction::cbr(cond_val, body_bb, exit_bb));
-        self.link_bb(cond_bb, body_bb);
-        self.link_bb(cond_bb, exit_bb);
+        // After linearizing condition, current_bb may be different from cond_bb
+        // (e.g., if condition contains short-circuit operators like && or ||).
+        // Link the CURRENT block to body_bb and exit_bb.
+        if let Some(cond_end_bb) = self.current_bb {
+            self.emit(Instruction::cbr(cond_val, body_bb, exit_bb));
+            self.link_bb(cond_end_bb, body_bb);
+            self.link_bb(cond_end_bb, exit_bb);
+        }
 
         // Body block
         self.break_targets.push(exit_bb);
@@ -1895,9 +1900,14 @@ impl<'a> Linearizer<'a> {
         // Condition block
         self.switch_bb(cond_bb);
         let cond_val = self.linearize_expr(cond);
-        self.emit(Instruction::cbr(cond_val, body_bb, exit_bb));
-        self.link_bb(cond_bb, body_bb);
-        self.link_bb(cond_bb, exit_bb);
+        // After linearizing condition, current_bb may be different from cond_bb
+        // (e.g., if condition contains short-circuit operators like && or ||).
+        // Link the CURRENT block to body_bb and exit_bb.
+        if let Some(cond_end_bb) = self.current_bb {
+            self.emit(Instruction::cbr(cond_val, body_bb, exit_bb));
+            self.link_bb(cond_end_bb, body_bb);
+            self.link_bb(cond_end_bb, exit_bb);
+        }
 
         // Exit block
         self.switch_bb(exit_bb);
@@ -1937,13 +1947,20 @@ impl<'a> Linearizer<'a> {
         self.switch_bb(cond_bb);
         if let Some(cond_expr) = cond {
             let cond_val = self.linearize_expr(cond_expr);
-            self.emit(Instruction::cbr(cond_val, body_bb, exit_bb));
+            // After linearizing condition, current_bb may be different from cond_bb
+            // (e.g., if condition contains short-circuit operators like && or ||).
+            // Link the CURRENT block to body_bb and exit_bb.
+            if let Some(cond_end_bb) = self.current_bb {
+                self.emit(Instruction::cbr(cond_val, body_bb, exit_bb));
+                self.link_bb(cond_end_bb, body_bb);
+                self.link_bb(cond_end_bb, exit_bb);
+            }
         } else {
             // No condition = always true
             self.emit(Instruction::br(body_bb));
+            self.link_bb(cond_bb, body_bb);
+            // No link to exit_bb since we always enter the body
         }
-        self.link_bb(cond_bb, body_bb);
-        self.link_bb(cond_bb, exit_bb);
 
         // Body block
         self.break_targets.push(exit_bb);