[cc] IR fixes

Summary of Fixes

  1. Block reordering for register allocator liveness (cc/ir/inline.rs)

  - Problem: After inlining, blocks were appended in arbitrary order to func.blocks. The register allocator's liveness analysis assumes blocks are in control flow order.
  - Fix: Added reorder_blocks_topologically() function that reorders blocks using BFS from the entry block after inlining completes.

  2. Inliner continuation block children detection (cc/ir/inline.rs)

  - Problem: When checking if a block branches to the continuation, the code checked only the last instruction. But if the original callee had return followed by __builtin_unreachable(), the unreachable was cloned after the branch, making the check fail.
  - Fix: Changed to check if any instruction in the block branches to continuation, not just the last.

  3. DCE treating Asm as root (cc/ir/dce.rs)

  - Problem: Opcode::Asm was not in the is_root() list, so DCE could eliminate inline assembly instructions.
  - Fix: Added Opcode::Asm to the root opcodes list.

  4. DCE tracking Asm input operands (cc/ir/dce.rs)

  - Problem: The get_uses() function didn't include pseudos from inline assembly inputs, so DCE could eliminate code that produces values used by inline assembly.
  - Fix: Added code to iterate through asm_data.inputs and add their pseudos to the uses list.

  5. Global symbol remapping for inliner (cc/ir/inline.rs - from earlier)

  - Added global_sym_map to ensure that when the same global symbol is referenced multiple times in an inlined function, they all map to the same caller pseudo ID.

Author: jgarzik

cc/ir/dce.rs

@@ -64,6 +64,7 @@ fn is_root(op: Opcode) -> bool {
             | Opcode::Alloca
             | Opcode::Setjmp  // Has side effects (saves context)
             | Opcode::Longjmp // Never returns (noreturn)
+            | Opcode::Asm // Inline assembly has side effects
     )
 }
 
@@ -91,20 +92,30 @@ fn get_uses(insn: &Instruction) -> Vec<PseudoId> {
         uses.push(indirect);
     }
 
+    // Inline assembly inputs (the pseudos that the asm reads)
+    if let Some(ref asm_data) = insn.asm_data {
+        for input in &asm_data.inputs {
+            uses.push(input.pseudo);
+        }
+    }
+
     uses
 }
 
-/// Find the instruction that defines a pseudo.
-/// Returns (block_index, instruction_index) if found.
-fn find_def(func: &Function, id: PseudoId) -> Option<(usize, usize)> {
+/// Find all instructions that define a pseudo.
+/// Returns vec of (block_index, instruction_index) for each definition.
+/// After inlining, a pseudo may have multiple definitions from different branches
+/// (e.g., the return target is written to from multiple return paths).
+fn find_all_defs(func: &Function, id: PseudoId) -> Vec<(usize, usize)> {
+    let mut defs = Vec::new();
     for (bb_idx, bb) in func.blocks.iter().enumerate() {
         for (insn_idx, insn) in bb.insns.iter().enumerate() {
             if insn.target == Some(id) {
-                return Some((bb_idx, insn_idx));
+                defs.push((bb_idx, insn_idx));
             }
         }
     }
-    None
+    defs
 }
 
 /// Eliminate dead code using mark-sweep algorithm.
@@ -128,8 +139,9 @@ fn eliminate_dead_code(func: &mut Function) -> bool {
 
     // Phase 2: Propagate liveness transitively
     while let Some(id) = worklist.pop_front() {
-        // Find the instruction that defines this pseudo
-        if let Some((bb_idx, insn_idx)) = find_def(func, id) {
+        // Find all instructions that define this pseudo
+        // (there may be multiple after inlining, e.g., return target written from multiple paths)
+        for (bb_idx, insn_idx) in find_all_defs(func, id) {
             let insn = &func.blocks[bb_idx].insns[insn_idx];
 
             // Mark all operands of the defining instruction as live

cc/ir/inline.rs

@@ -200,6 +200,9 @@ struct InlineContext {
     pseudo_map: HashMap<PseudoId, PseudoId>,
     /// Map from callee BasicBlockId to caller BasicBlockId
     bb_map: HashMap<BasicBlockId, BasicBlockId>,
+    /// Map from global symbol names to their caller pseudo IDs
+    /// Multiple callee pseudos with the same global symbol name should map to the same caller pseudo
+    global_sym_map: HashMap<String, PseudoId>,
     /// Next pseudo ID available in the caller
     next_pseudo_id: u32,
     /// Next basic block ID available in the caller
@@ -239,8 +242,8 @@ impl InlineContext {
         return_continuation_bb: BasicBlockId,
         return_target: Option<PseudoId>,
     ) -> Self {
-        // Find max pseudo ID and BB ID in caller
-        let max_pseudo = caller.pseudos.iter().map(|p| p.id.0).max().unwrap_or(0);
+        // Use caller.next_pseudo instead of scanning pseudos list
+        // (the list may not contain all pseudo IDs, e.g., phi nodes from SSA)
         let max_bb = caller.blocks.iter().map(|b| b.id.0).max().unwrap_or(0);
 
         // Collect callee's local variable names - these need to be renamed
@@ -249,7 +252,8 @@ impl InlineContext {
         Self {
             pseudo_map: HashMap::new(),
             bb_map: HashMap::new(),
-            next_pseudo_id: max_pseudo + 1,
+            global_sym_map: HashMap::new(),
+            next_pseudo_id: caller.next_pseudo,
             next_bb_id: max_bb + 1,
             call_args,
             return_continuation_bb,
@@ -308,6 +312,18 @@ impl InlineContext {
                     self.alloc_pseudo_id()
                 }
             }
+            // Global symbols: ensure same name maps to same ID across multiple inlinings
+            // This is critical when a function is inlined multiple times and references
+            // the same global variable - all references must point to the same pseudo
+            Some(PseudoKind::Sym(name)) if !self.callee_locals.contains(name) => {
+                if let Some(&existing_id) = self.global_sym_map.get(name) {
+                    self.pseudo_map.insert(callee_id, existing_id);
+                    return existing_id;
+                }
+                let new_id = self.alloc_pseudo_id();
+                self.global_sym_map.insert(name.clone(), new_id);
+                new_id
+            }
             // For all other kinds, allocate new ID
             _ => self.alloc_pseudo_id(),
         };
@@ -636,12 +652,14 @@ fn inline_call_site(
         }
 
         // Blocks with Ret (now Br to continuation): update children
-        if block
+        // Note: We check for ANY instruction (not just last) that branches to continuation,
+        // because the original callee block may have had unreachable instructions after ret
+        // (e.g., __builtin_unreachable() after return), which get cloned after the branch.
+        let has_branch_to_continuation = block
             .insns
-            .last()
-            .map(|i| i.op == Opcode::Br && i.bb_true == Some(continuation_bb_id))
-            .unwrap_or(false)
-        {
+            .iter()
+            .any(|i| i.op == Opcode::Br && i.bb_true == Some(continuation_bb_id));
+        if has_branch_to_continuation {
             block.children = vec![continuation_bb_id];
             continuation_bb.parents.push(block.id);
         }
@@ -681,20 +699,96 @@ fn inline_call_site(
         }
     }
 
-    // Update old children's parent references to point to continuation
+    // Update old children's parent references and phi nodes to point to continuation
     for &child_id in &old_children {
         if let Some(child) = caller.blocks.iter_mut().find(|b| b.id == child_id) {
+            // Update parent list
             for parent in &mut child.parents {
                 if *parent == call_bb_id {
                     *parent = continuation_bb_id;
                 }
             }
+
+            // Update phi nodes: replace call_bb_id with continuation_bb_id
+            for insn in &mut child.insns {
+                if insn.op == Opcode::Phi {
+                    for (pred_bb, _) in &mut insn.phi_list {
+                        if *pred_bb == call_bb_id {
+                            *pred_bb = continuation_bb_id;
+                        }
+                    }
+                }
+            }
         }
     }
 
+    // Update caller's next_pseudo to avoid ID collisions with later allocations
+    caller.next_pseudo = ctx.next_pseudo_id;
+
     true
 }
 
+// ============================================================================
+// Block Reordering (for correct liveness analysis in regalloc)
+// ============================================================================
+
+/// Reorder blocks in control flow (topological) order.
+/// This is critical for the linear scan register allocator, which assumes
+/// blocks are in execution order when computing live intervals.
+fn reorder_blocks_topologically(func: &mut Function) {
+    if func.blocks.is_empty() {
+        return;
+    }
+
+    // Build a map of block ID to block
+    let block_map: HashMap<BasicBlockId, usize> = func
+        .blocks
+        .iter()
+        .enumerate()
+        .map(|(i, b)| (b.id, i))
+        .collect();
+
+    // BFS from entry to get reachable blocks in control flow order
+    let mut visited: HashSet<BasicBlockId> = HashSet::new();
+    let mut order: Vec<BasicBlockId> = Vec::new();
+    let mut queue: std::collections::VecDeque<BasicBlockId> = std::collections::VecDeque::new();
+
+    queue.push_back(func.entry);
+
+    while let Some(bb_id) = queue.pop_front() {
+        if visited.contains(&bb_id) {
+            continue;
+        }
+        visited.insert(bb_id);
+        order.push(bb_id);
+
+        // Get successors from the block's children
+        if let Some(&idx) = block_map.get(&bb_id) {
+            for &child_id in &func.blocks[idx].children {
+                if !visited.contains(&child_id) {
+                    queue.push_back(child_id);
+                }
+            }
+        }
+    }
+
+    // Include any unreachable blocks at the end (shouldn't happen, but be safe)
+    for block in &func.blocks {
+        if !visited.contains(&block.id) {
+            order.push(block.id);
+        }
+    }
+
+    // Reorder blocks according to the computed order
+    let mut new_blocks: Vec<BasicBlock> = Vec::with_capacity(func.blocks.len());
+    for bb_id in order {
+        if let Some(&idx) = block_map.get(&bb_id) {
+            new_blocks.push(func.blocks[idx].clone());
+        }
+    }
+    func.blocks = new_blocks;
+}
+
 // ============================================================================
 // Main Inlining Pass
 // ============================================================================
@@ -768,6 +862,15 @@ pub fn run(module: &mut Module, opt_level: u32) -> bool {
         }
     }
 
+    // Reorder blocks in all functions that were modified
+    // This ensures blocks are in control flow order for the register allocator's
+    // linear scan algorithm, which relies on position-based liveness analysis
+    if any_changed {
+        for func in &mut module.functions {
+            reorder_blocks_topologically(func);
+        }
+    }
+
     // Remove dead static functions that were fully inlined
     if any_changed {
         remove_dead_functions(module);

cc/ir/instcombine.rs

@@ -612,10 +612,14 @@ mod tests {
         let types = TypeTable::new(64);
         let mut func = Function::new("test", types.int_id);
 
-        for p in pseudos {
-            func.add_pseudo(p);
+        for p in &pseudos {
+            func.add_pseudo(p.clone());
         }
 
+        // Set next_pseudo to be after the highest pseudo ID
+        let max_id = pseudos.iter().map(|p| p.id.0).max().unwrap_or(0);
+        func.next_pseudo = max_id + 1;
+
         let mut bb = BasicBlock::new(BasicBlockId(0));
         bb.add_insn(Instruction::new(Opcode::Entry));
         bb.add_insn(insn);

cc/ir/linearize.rs

@@ -1050,6 +1050,12 @@ impl<'a> Linearizer<'a> {
         if self.run_ssa {
             if let Some(ref mut ir_func) = self.current_func {
                 ssa_convert(ir_func, self.types);
+                // Note: ssa_convert sets ir_func.next_pseudo to account for phi nodes
+            }
+        } else {
+            // Only set next_pseudo if SSA was NOT run (SSA sets its own)
+            if let Some(ref mut ir_func) = self.current_func {
+                ir_func.next_pseudo = self.next_pseudo;
             }
         }
 

cc/ir/mod.rs

@@ -1054,6 +1054,8 @@ pub struct Function {
     pub entry: BasicBlockId,
     /// All pseudos indexed by PseudoId
     pub pseudos: Vec<Pseudo>,
+    /// Next pseudo ID to allocate (monotonically increasing)
+    pub next_pseudo: u32,
     /// Local variables (name -> info), used for SSA conversion
     pub locals: HashMap<String, LocalVar>,
     /// Maximum dominator tree depth (computed by dominate.rs)
@@ -1075,6 +1077,7 @@ impl Default for Function {
             blocks: Vec::new(),
             entry: BasicBlockId(0),
             pseudos: Vec::new(),
+            next_pseudo: 0,
             locals: HashMap::new(),
             max_dom_level: 0,
             is_static: false,
@@ -1144,17 +1147,18 @@ impl Function {
         self.locals.get(name)
     }
 
-    /// Compute the next available pseudo ID
-    /// This scans all existing pseudos to find the maximum ID, then returns max + 1
-    pub fn next_pseudo_id(&self) -> PseudoId {
-        let max_id = self.pseudos.iter().map(|p| p.id.0).max().unwrap_or(0);
-        PseudoId(max_id + 1)
+    /// Allocate a new pseudo ID
+    /// Returns a unique ID and increments the counter
+    pub fn alloc_pseudo(&mut self) -> PseudoId {
+        let id = PseudoId(self.next_pseudo);
+        self.next_pseudo += 1;
+        id
     }
 
     /// Create a new constant integer pseudo and return its ID
     /// The pseudo is added to self.pseudos
     pub fn create_const_pseudo(&mut self, value: i64) -> PseudoId {
-        let id = self.next_pseudo_id();
+        let id = self.alloc_pseudo();
         let pseudo = Pseudo::val(id, value);
         self.add_pseudo(pseudo);
         id

cc/ir/ssa.rs

@@ -666,6 +666,9 @@ pub fn ssa_convert(func: &mut Function, types: &TypeTable) {
     // Phase 3: Remove dead stores
     let dead_stores = std::mem::take(&mut converter.dead_stores);
     remove_dead_stores(converter.func, &dead_stores);
+
+    // Update function's next_pseudo to avoid ID collisions with later allocations
+    converter.func.next_pseudo = converter.next_pseudo_id;
 }
 
 // ============================================================================