[CodeGenPrepare] Failure to hoist bitcast to legal type causes register splitting

Project: llvm/llvm-project | Language: C++ | Concepts: algorithmic-optimization, data-structures | Difficulty: senior
Link: https://github.com/llvm/llvm-project/pull/207568

The problem

LLVM's CodeGenPrepare pass did not hoist certain bitcast instructions to the source block when doing so would have reduced register pressure and prevented inefficient register splitting. This could lead to suboptimal code generation, especially when casting between types that require different numbers of physical registers, resulting in unnecessary register splits and spills.

Why it's non-trivial

A naive approach might hoist all bitcast instructions or only consider type legality, but this can break correctness or introduce new inefficiencies. For example, hoisting must be avoided for:

• Scalable vector types (e.g., RISC-V V extension types).
• Bitcasts that would increase register pressure or cross register domains (e.g., integer-to-float conversions for large scalars).
• Cases involving atomics, which have strict requirements on instruction placement for correctness.

The approach

The PR introduces a dedicated function, optimizeBitCast, to encapsulate the logic for safely hoisting bitcast instructions. Key decision points are guarded by explicit checks:

+static bool optimizeBitCast(BitCastInst *BCI, const TargetLowering &TLI,
+                            const DataLayout &DL) {
+  auto *SrcInst = dyn_cast<Instruction>(BCI->getOperand(0));
+  if (!SrcInst || SrcInst->getParent() == BCI->getParent() ||
+      SrcInst->isTerminator())
+    return false;

This ensures only bitcasts whose source is an instruction in a different block (and not a terminator) are considered for hoisting.

+  EVT SrcVT = TLI.getValueType(DL, SrcInst->getType());
+  EVT DestVT = TLI.getValueType(DL, BCI->getType());
+
+  // Bail out on scalable vectors and illegal destination types
+  if (SrcVT.isScalableVector() || DestVT.isScalableVector() ||
+      !TLI.isTypeLegal(DestVT))
+    return false;

Prevents hoisting for scalable vectors (which have special handling on some targets) and illegal destination types.

+  // Only hoist if it reduces physical register count
+  if (TLI.getNumRegisters(BCI->getContext(), SrcVT) <=
+      TLI.getNumRegisters(BCI->getContext(), DestVT))
+    return false;

Ensures hoisting only happens if it will reduce the number of physical registers needed, which reduces register pressure.

+  // Prevent large cross-domain scalar hoists
+  Type *DestTy = BCI->getType();
+  Type *SrcTy = SrcInst->getType();
+  bool IsCrossDomain = DestTy->isFPOrFPVectorTy() != SrcTy->isFPOrFPVectorTy();
+  bool IsLargeScalar = !DestTy->isVectorTy() &&
+                       DL.getTypeSizeInBits(DestTy).getFixedValue() > 64;
+  if (IsCrossDomain && IsLargeScalar)
+    return false;

Avoids hoisting bitcasts that cross register domains (e.g., int-to-float) for large scalars, which could cause spills or break atomic operations.

+  // SelectionDAG Guard to prevent breaking atomic loop layout
+  auto IsAtomic = [](const Value *V) {
+    const auto *I = dyn_cast<Instruction>(V);
+    return I && I->isAtomic();
+  };
+
+  // Check upstream atomic -> [extractvalue] -> bitcast
+  const Value *Origin = SrcInst;
+  if (auto *EV = dyn_cast<ExtractValueInst>(Origin))
+    Origin = EV->getAggregateOperand();
+
+  // Check downstream bitcast -> [phi] -> atomic
+  if (IsAtomic(Origin) || any_of(BCI->users(), [IsAtomic](const User *U) {
+        return IsAtomic(U) || (isa<PHINode>(U) && any_of(U->users(), IsAtomic));
+      }))
+    return false;

Guards against moving bitcasts in ways that would disrupt atomic instruction semantics, both upstream and downstream.

+  // Hoist the bitcast
+  BasicBlock *SrcBB = SrcInst->getParent();
+  auto InsertPt = isa<PHINode>(SrcInst) ? SrcBB->getFirstInsertionPt()
+                                        : std::next(SrcInst->getIterator());
+  BCI->moveBefore(*SrcBB, InsertPt);
+  return true;
+}

Performs the actual hoisting by moving the bitcast immediately after its source instruction (or at the start of the block if the source is a PHI node).

The function is then called from optimizeInst:

+    if (auto *BCI = dyn_cast<BitCastInst>(CI)) {
+      // Hoist bitcasts of illegal types to reduce cross-block register pressure
+      // and prevent register splitting.
+      if (optimizeBitCast(BCI, *TLI, *DL)) {
+        return true;        
+      }
+    }

This integrates the new logic into the main optimization pass.

The PR also adds targeted tests for AArch64, RISC-V, and x86 to verify the correct hoisting and non-hoisting of bitcasts in various scenarios.

From the review thread

No notable review discussion on this PR.

Takeaways

• Only perform code motion when it reduces resource usage and preserves correctness. Hoisting should be conditional on reducing register pressure and must not break atomicity or type legality.
• Guard against special cases (e.g., scalable vectors, atomics, cross-domain casts) that have non-obvious constraints. Always check for target-specific and semantic constraints before transforming IR.
• Encapsulate complex transformation logic in a dedicated function for clarity and maintainability. This makes it easier to reason about and test the transformation, and to extend or modify it in the future.