Fix GVN and SROA miscompilation of min precision vector element access

[alsepkow]

Summary

Fixes three related optimizer bugs that cause miscompilation of min precision vector element access ([] operator) on min16float, min16int, and min16uint types at optimization levels O1+.

Resolves #8268

Root Cause

DXC's data layout pads min precision types (i16:32, f16:32). The HLSL change in DataLayout::getTypeSizeInBits makes vector sizes use alloc size per element (e.g., <3 x i16> = 96 bits), but scalar getTypeSizeInBits(i16) returns the primitive width (16 bits). This inconsistency causes three bugs:

1. GVN ICE: CanCoerceMustAliasedValueToLoad creates a padded-width integer (i96) then attempts a bitcast from the 48-bit LLVM vector type — assert fires.
2. GVN incorrect store forwarding: processLoad forwards a zeroinitializer store past partial element stores because MemoryDependence uses padded sizes for aliasing.
3. SROA element misindexing (primary bug): getNaturalGEPRecursively uses getTypeSizeInBits(i16)/8 = 2 for element offsets, while GEP uses getTypeAllocSize(i16) = 4. Byte offset 4 (element 1) maps to index 4/2 = 2 instead of 4/4 = 1, causing SROA to misplace or eliminate element stores.

Changes

lib/Transforms/Scalar/GVN.cpp

• CanCoerceMustAliasedValueToLoad: Reject coercion when type sizes include padding (getTypeSizeInBits != getPrimitiveSizeInBits)
• processLoad StoreInst handler: Skip store-to-load forwarding for padded types

lib/Transforms/Scalar/SROA.cpp

• getNaturalGEPRecursively: Use getTypeAllocSizeInBits for vector element size to match GEP offset stride
• isVectorPromotionViable: Same fix for element size calculation
• AllocaSliceRewriter constructor: Same fix for ElementSize

Testing

• All 6 min precision ArrayOperator tests (StaticAccess + DynamicAccess × 3 types) pass with the fix
• Verified optimized DXIL output retains all 3 element stores with correct indices

Co-authored

This fix was investigated and implemented with the assistance of GitHub Copilot (AI pair programming). The root cause analysis — tracing the bug through -print-after-all pass dumps, identifying SROA as the culprit, and understanding the getTypeSizeInBits vs getTypeAllocSize mismatch — was a collaborative effort.

Co-authored-by: Copilot 223556219+Copilot@users.noreply.github.com

[github-actions]

✅ With the latest revision this PR passed the C/C++ code formatter.

[Copilot]

Pull request overview

Fixes optimizer miscompilations affecting min-precision vector element ([]) access under padded HLSL data layouts by aligning GVN/SROA behavior with allocation-size-based strides and preventing unsafe GVN coercions/forwarding.

Changes:

• Update SROA vector element stride calculations to use allocation size in bits (matching GEP byte offset stride under padded layouts).
• Harden GVN against coercion and cross-type store-to-load forwarding when data layout padding would make the transformation invalid.
• Add new regression tests covering SROA and GVN behavior for padded i16/half vectors, including partial element stores.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.

File	Description
test/Transforms/SROA/min-precision-padding.ll	Adds SROA regression coverage ensuring vector element GEP indexing matches padded element stride.
test/Transforms/GVN/min-precision-padding.ll	Adds GVN regression coverage preventing unsafe coercion/forwarding with padded min-precision vectors while preserving same-type forwarding.
lib/Transforms/Scalar/SROA.cpp	Switches vector element sizing to alloc-size-in-bits in multiple SROA decision points.
lib/Transforms/Scalar/GVN.cpp	Rejects coercion and adds forwarding guardrails when type size implies padding-related hazards.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[V-FEXrt]

Overall LGTM but this part of the code base (min precision) is notorious and I would really like someone whose worked on it to review. Otherwise a couple smaller comment

also, I've notice that code generated by LLMs seem to way over comment compared to code a human would write. A lot of these comments are fairly repetitive (I think I've seen "the issue is 2/1 is 2 not 1" at least 4 times in the PR). Not sure what we should do about it but in general I prefer for "why not how" type comments and LLMs seem to create a lot of "how" comments (I assume for the benefit of the human-in-the-loop?) Maybe we should have a editorial policy here if its a common trope others notice

[damyanp]

This LGTM, and I did some poking to see if these changes might impact non-min-precision types and convinced myself it wouldn't.

We should get an approval from someone more familiar with this area of the code though.

[tex3d]

I think this looks good. I do wonder if there are other places with a similar assumption (that vector width is dependent on element size, not element alloc size), but this certainly seems to fix things without regressing anything as far as I can tell.