[llvm] [DAGCombiner] Restrict mergeTruncStores to pre-type-legalization combines (PR #222245)

Tue Sep 8 23:03:01 PDT 2026

llvmorg-github-actions[bot] wrote:




@llvm/pr-subscribers-backend-aarch64

Author: Chung-Yi Chen (ADNRs)

<details>
<summary>Changes</summary>

Fixes #222231.

`mergeTruncStores()` merges adjacent narrow stores of the pieces of one wide value into a single wide store, creating a `TRUNCATE` to the wide type and, when the byte order is reversed, a `BSWAP` or `ROTR` of it. Introducing those nodes with an illegal type is intentional, but only while types have not been legalized yet.

The fold is guarded by `LegalOperations`, which is set one combine round later (`Level >= AfterLegalizeVectorOps`). It therefore also runs in the `AfterLegalizeTypes` round, where new nodes must already have legal types, and the illegal nodes then reach `SelectionDAGLegalize::LegalizeOp` and trip its `"Unexpected illegal type!"` assertion. Without assertions they survive legalization and instruction selection fails with `"Cannot select"`.

The fix is to guard on `LegalTypes` instead, which is what 1d0fa798248f ("[DAGCombiner] restrict store merge of truncs to early combining") intended:

> The pattern matching does not account for truncating stores, so it is unlikely to work at later stages. So we are likely wasting compile-time with no hope of improvement by running this later.

The pattern *can* still match after type legalization when it only becomes visible there. In the added test the stored bytes are forwarded from earlier stores, so the fold first sees the pair in the `AfterLegalizeTypes` round and merges two `i8` stores into an `i16` store, which is an illegal type on AArch64 (and on WebAssembly).

## Alternative considered

The fold could instead keep running after type legalization and bail out only on an illegal wide type (`if (LegalTypes && !TLI.isTypeLegal(WideVT))`), which would preserve late merges whose wide type *is* legal. That preserves nothing measurable: instrumenting the fold and running `llc` over every `.ll` file under `llvm/test/CodeGen` (all 23 target directories, each with its natural triple) gives 59 firings, 58 of them in the `BeforeLegalizeTypes` round. The single `AfterLegalizeTypes` firing is the crash fixed here, and its wide type is illegal. Moving the guard is also strictly cheaper, since the fold no longer walks store chains in a round where it has never produced a valid result.

## Testing

* Added a regression test to `llvm/test/CodeGen/AArch64/merge-trunc-store.ll`, the existing test file for this fold, which already covers little- and big-endian AArch64. Checks were generated with `update_llc_test_checks.py`; the diff adds lines only, no existing check line changed. The test fails on a pre-patch build and passes with the patch.
* `check-llvm`: 77998 tests, no unexpected failures.
* Verified the reproducer no longer crashes on all 43 target triples that compile it.


---
Full diff: https://github.com/llvm/llvm-project/pull/222245.diff


2 Files Affected:

- (modified) llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp (+5-1) 
- (modified) llvm/test/CodeGen/AArch64/merge-trunc-store.ll (+33) 


``````````diff

diff --git a/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp b/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
index 733d0eb9baa40..b73d447a90766 100644
--- a/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp
@@ -9946,10 +9946,14 @@ static SDValue stripTruncAndExt(SDValue Value) {
 SDValue DAGCombiner::mergeTruncStores(StoreSDNode *N) {
   // The matching looks for "store (trunc x)" patterns that appear early but are
   // likely to be replaced by truncating store nodes during combining.
+  // This must be restricted to the combine rounds that run before type
+  // legalization: the fold creates a truncate to the wide type plus an optional
+  // bswap/rotate of it, and those are only allowed to have an illegal type
+  // while types have not been legalized yet.
   // TODO: If there is evidence that running this later would help, this
   //       limitation could be removed. Legality checks may need to be added
   //       for the created store and optional bswap/rotate.
-  if (LegalOperations || OptLevel == CodeGenOptLevel::None)
+  if (LegalTypes || OptLevel == CodeGenOptLevel::None)
     return SDValue();
 
   // We only handle merging simple stores of 1-4 bytes.
diff --git a/llvm/test/CodeGen/AArch64/merge-trunc-store.ll b/llvm/test/CodeGen/AArch64/merge-trunc-store.ll
index b161d746ad11d..78b56db923f4e 100644
--- a/llvm/test/CodeGen/AArch64/merge-trunc-store.ll
+++ b/llvm/test/CodeGen/AArch64/merge-trunc-store.ll
@@ -839,3 +839,36 @@ define void @i32_to_i8_wrong_order(i32 %x, ptr %p0) {
   store i8 %t1, ptr %p2, align 1
   ret void
 }
+
+; Negative test - the pattern only becomes visible after type legalization here,
+; because the stored bytes are forwarded from the earlier stores. Merging would
+; need an i16 truncate/bswap, and i16 is not a legal type, so the fold must not
+; run this late.
+
+define void @merge_i16_illegal_after_type_legalization(i64 %x, ptr %p, ptr noalias %q) {
+; CHECK-LABEL: merge_i16_illegal_after_type_legalization:
+; CHECK:       // %bb.0:
+; CHECK-NEXT:    mov x8, xzr
+; CHECK-NEXT:    lsr x9, x0, #8
+; CHECK-NEXT:    strb w0, [x2]
+; CHECK-NEXT:    ldr x8, [x8]
+; CHECK-NEXT:    strb w9, [x1]
+; CHECK-NEXT:    strb w9, [x8]
+; CHECK-NEXT:    strb w0, [x8, #1]
+; CHECK-NEXT:    strb wzr, [x1]
+; CHECK-NEXT:    ret
+  %b = load ptr, ptr null, align 8
+  %sh = lshr i64 %x, 8
+  %t1 = trunc i64 %sh to i8
+  store i8 %t1, ptr %p, align 1
+  %fwd1 = load i8, ptr %p, align 1
+  store i8 %fwd1, ptr %b, align 1
+  %t0 = trunc i64 %x to i8
+  store i8 %t0, ptr %q, align 1
+  %b2 = load ptr, ptr null, align 8
+  %b2p1 = getelementptr i8, ptr %b2, i64 1
+  %fwd0 = load i8, ptr %q, align 1
+  store i8 %fwd0, ptr %b2p1, align 1
+  store i8 0, ptr %p, align 1
+  ret void
+}

``````````

</details>


https://github.com/llvm/llvm-project/pull/222245
