From c53e2610900b52ce2936bfc8b62d2527f4353c35 Mon Sep 17 00:00:00 2001 From: Vladislav Korytsko Date: Sun, 23 Aug 2026 23:04:39 +0300 Subject: [PATCH 1/2] Fix dropped inout write-back when casting between identical layout structs Casting a struct to an identical-layout struct in an inout argument makes CodeGen coerce it with a pointer bitcast. SROA_Parameter_HLSL replaced that bitcast with a one-way copy into a fresh alloca, so every write the callee made was silently discarded. That copy prepass was introduced in #1718 to fix a crash. #2745 later taught RewriteBitCast to replace the cast with its operand, keeping the aliasing copy-out needs. The prepass ran first and broke it. Redundant prepass removed. --- .../Scalar/ScalarReplAggregatesHLSL.cpp | 42 ------------------- .../cast_as_func_inout_param-strictudt.hlsl | 42 +++++++++++++++++++ .../cast_as_func_inout_param.hlsl | 42 +++++++++++++++++++ 3 files changed, 84 insertions(+), 42 deletions(-) create mode 100644 tools/clang/test/HLSLFileCheck/hlsl/types/cast/identical_layout_structs/cast_as_func_inout_param-strictudt.hlsl create mode 100644 tools/clang/test/HLSLFileCheck/hlsl/types/cast/identical_layout_structs/cast_as_func_inout_param.hlsl diff --git a/lib/Transforms/Scalar/ScalarReplAggregatesHLSL.cpp b/lib/Transforms/Scalar/ScalarReplAggregatesHLSL.cpp index d80a678651..ba2dad8f06 100644 --- a/lib/Transforms/Scalar/ScalarReplAggregatesHLSL.cpp +++ b/lib/Transforms/Scalar/ScalarReplAggregatesHLSL.cpp @@ -4395,8 +4395,6 @@ class SROA_Parameter_HLSL : public ModulePass { static char ID; // Pass identification, replacement for typeid explicit SROA_Parameter_HLSL() : ModulePass(ID) {} StringRef getPassName() const override { return "SROA Parameter HLSL"; } - static void RewriteBitcastWithIdenticalStructs(Function *F); - static void RewriteBitcastWithIdenticalStructs(BitCastInst *BCI); static bool DeleteSimpleStoreOnlyAlloca(AllocaInst *AI); static bool IsSimpleStoreOnlyAlloca(AllocaInst *AI); @@ -4464,7 +4462,6 @@ class SROA_Parameter_HLSL : public ModulePass { while (!WorkList.empty()) { Function *F = WorkList.front(); WorkList.pop_front(); - RewriteBitcastWithIdenticalStructs(F); createFlattenedFunction(F); } @@ -4609,28 +4606,6 @@ INITIALIZE_PASS(SROA_Parameter_HLSL, "scalarrepl-param-hlsl", "Scalar Replacement of Aggregates HLSL (parameters)", false, false) -void SROA_Parameter_HLSL::RewriteBitcastWithIdenticalStructs(Function *F) { - if (F->isDeclaration()) - return; - // Gather list of bitcast involving src and dest structs with identical layout - std::vector worklist; - for (inst_iterator I = inst_begin(F), E = inst_end(F); I != E; ++I) { - if (BitCastInst *BCI = dyn_cast(&*I)) { - Type *DstTy = BCI->getDestTy(); - Type *SrcTy = BCI->getSrcTy(); - if (ArePointersToStructsOfIdenticalLayouts(DstTy, SrcTy)) - worklist.push_back(BCI); - } - } - - // Replace bitcast involving src and dest structs with identical layout - while (!worklist.empty()) { - BitCastInst *BCI = worklist.back(); - worklist.pop_back(); - RewriteBitcastWithIdenticalStructs(BCI); - } -} - bool SROA_Parameter_HLSL::IsSimpleStoreOnlyAlloca(AllocaInst *AI) { if (!AI->getAllocatedType()->isSingleValueType()) return false; @@ -4661,23 +4636,6 @@ bool SROA_Parameter_HLSL::DeleteSimpleStoreOnlyAlloca(AllocaInst *AI) { return true; } -void SROA_Parameter_HLSL::RewriteBitcastWithIdenticalStructs(BitCastInst *BCI) { - StructType *srcStTy = - cast(BCI->getSrcTy()->getPointerElementType()); - StructType *destStTy = - cast(BCI->getDestTy()->getPointerElementType()); - Value *srcPtr = BCI->getOperand(0); - IRBuilder<> AllocaBuilder( - dxilutil::FindAllocaInsertionPt(BCI->getParent()->getParent())); - AllocaInst *destPtr = AllocaBuilder.CreateAlloca(destStTy); - IRBuilder<> InstBuilder(BCI); - std::vector idxlist = {0}; - CopyElementsOfStructsWithIdenticalLayout(InstBuilder, destPtr, srcPtr, - srcStTy, idxlist); - BCI->replaceAllUsesWith(destPtr); - BCI->eraseFromParent(); -} - /// DeleteDeadInstructions - Erase instructions on the DeadInstrs list, /// recursively including all their operands that become trivially dead. void SROA_Parameter_HLSL::DeleteDeadInstructions() { diff --git a/tools/clang/test/HLSLFileCheck/hlsl/types/cast/identical_layout_structs/cast_as_func_inout_param-strictudt.hlsl b/tools/clang/test/HLSLFileCheck/hlsl/types/cast/identical_layout_structs/cast_as_func_inout_param-strictudt.hlsl new file mode 100644 index 0000000000..e989f6a51a --- /dev/null +++ b/tools/clang/test/HLSLFileCheck/hlsl/types/cast/identical_layout_structs/cast_as_func_inout_param-strictudt.hlsl @@ -0,0 +1,42 @@ +// RUN: %dxc -T ps_6_0 -HV 2021 %s | FileCheck %s +// RUN: %dxc -T ps_6_0 -HV 2021 -DSTATIC_GLOBAL %s | FileCheck %s + +// Validate that the copy-out of an inout argument survives a cast between two +// structs of identical layout. + +// CHECK: call void @dx.op.storeOutput.f32(i32 5, i32 0, i32 0, i8 0, float 1.000000e+00) +// CHECK: call void @dx.op.storeOutput.f32(i32 5, i32 0, i32 0, i8 1, float 2.000000e+00) + +struct S { + float a; + int b; +}; + +struct T { + float c; + int d; +}; + +void f(inout S s) { + s.a = 1; + s.b = 2; +} + +#ifdef STATIC_GLOBAL + +static T g; + +float4 main() : SV_Target { + f((S)g); + return float4(g.c, g.d, 0, 0); +} + +#else + +float4 main() : SV_Target { + T t = (T)0; + f((S)t); + return float4(t.c, t.d, 0, 0); +} + +#endif diff --git a/tools/clang/test/HLSLFileCheck/hlsl/types/cast/identical_layout_structs/cast_as_func_inout_param.hlsl b/tools/clang/test/HLSLFileCheck/hlsl/types/cast/identical_layout_structs/cast_as_func_inout_param.hlsl new file mode 100644 index 0000000000..b8d54a11a5 --- /dev/null +++ b/tools/clang/test/HLSLFileCheck/hlsl/types/cast/identical_layout_structs/cast_as_func_inout_param.hlsl @@ -0,0 +1,42 @@ +// RUN: %dxc -T ps_6_0 -HV 2018 %s | FileCheck %s +// RUN: %dxc -T ps_6_0 -HV 2018 -DSTATIC_GLOBAL %s | FileCheck %s + +// Validate that the copy-out of an inout argument survives a cast between two +// structs of identical layout. + +// CHECK: call void @dx.op.storeOutput.f32(i32 5, i32 0, i32 0, i8 0, float 1.000000e+00) +// CHECK: call void @dx.op.storeOutput.f32(i32 5, i32 0, i32 0, i8 1, float 2.000000e+00) + +struct S { + float a; + int b; +}; + +struct T { + float c; + int d; +}; + +void f(inout S s) { + s.a = 1; + s.b = 2; +} + +#ifdef STATIC_GLOBAL + +static T g; + +float4 main() : SV_Target { + f(g); + return float4(g.c, g.d, 0, 0); +} + +#else + +float4 main() : SV_Target { + T t = (T)0; + f(t); + return float4(t.c, t.d, 0, 0); +} + +#endif From 839b33cbcb489ae180f0ecae6d2cb7eb7acb367b Mon Sep 17 00:00:00 2001 From: Vladislav Korytsko Date: Sun, 23 Aug 2026 23:16:06 +0300 Subject: [PATCH 2/2] Fix compiler hang casting a struct to a different-layout struct Passing a struct to an inout parameter of a different-layout struct makes CodeGen coerce it with a pointer bitcast. RewriteBitCast has no rewrite for such a pair and returned with the bitcast still using the value being replaced, so RewriteForScalarRepl spun on a use it could not eliminate. #2745 fixed identical-layouts, not these. Keep such an alloca out of scalar replacement, leaving the inout copy-out intact, and diagnose what cannot be (like a static global). --- .../Scalar/ScalarReplAggregatesHLSL.cpp | 40 ++++++++++++++++++- .../cast_as_func_inout_param-strictudt.hlsl | 40 +++++++++++++++++++ .../cast_as_func_inout_param.hlsl | 39 ++++++++++++++++++ 3 files changed, 118 insertions(+), 1 deletion(-) create mode 100644 tools/clang/test/HLSLFileCheck/hlsl/types/cast/similar_layout_structs/cast_as_func_inout_param-strictudt.hlsl create mode 100644 tools/clang/test/HLSLFileCheck/hlsl/types/cast/similar_layout_structs/cast_as_func_inout_param.hlsl diff --git a/lib/Transforms/Scalar/ScalarReplAggregatesHLSL.cpp b/lib/Transforms/Scalar/ScalarReplAggregatesHLSL.cpp index ba2dad8f06..dcd16fb43c 100644 --- a/lib/Transforms/Scalar/ScalarReplAggregatesHLSL.cpp +++ b/lib/Transforms/Scalar/ScalarReplAggregatesHLSL.cpp @@ -1533,6 +1533,39 @@ static bool isUDTIntrinsicArg(CallInst *CI, unsigned OpIdx) { return false; } +/// isSafeBitCastForScalarRepl - Check if a bitcast can be handled for scalar +/// replacement. A struct to struct pointer cast is only rewritable when the +/// destination is reachable through the leading elements of the source, or +/// when both structs have an identical layout. +static bool isSafeBitCastForScalarRepl(BitCastInst *BCI) { + // Unused bitcast may be leftover from temporary memcpy + if (BCI->use_empty()) + return true; + + Type *DstTy = BCI->getType(); + Type *SrcTy = BCI->getOperand(0)->getType(); + + if (!DstTy->isPointerTy() || !SrcTy->isPointerTy()) + return true; + + StructType *DstST = dyn_cast(DstTy->getPointerElementType()); + StructType *SrcST = dyn_cast(SrcTy->getPointerElementType()); + + // A non-struct destination is rewritten per llvm.lifetime.* intrinsic user + // A non-struct source never reaches the struct to struct rewrite + if (!DstST || !SrcST) + return true; + + for (StructType *ST = SrcST; ST && ST->getNumElements();) { + Type *EltTy = ST->getElementType(0); + if (EltTy == DstST) + return true; + ST = dyn_cast(EltTy); + } + + return SrcST->isLayoutIdentical(DstST); +} + /// isSafeForScalarRepl - Check if instruction I is a safe use with regard to /// performing scalar replacement of alloca AI. The results are flagged in /// the Info parameter. Offset indicates the position within AI that is @@ -1548,6 +1581,8 @@ void isSafeForScalarRepl(Instruction *I, uint64_t Offset, AllocaInfo &Info) { Instruction *User = cast(U.getUser()); if (BitCastInst *BC = dyn_cast(User)) { + if (!isSafeBitCastForScalarRepl(BC)) + return MarkUnsafe(Info, User); isSafeForScalarRepl(BC, Offset, Info); } else if (GetElementPtrInst *GEPI = dyn_cast(User)) { uint64_t GEPOffset = Offset; @@ -2687,7 +2722,10 @@ void SROA_Helper::RewriteBitCast(BitCastInst *BCI) { BCI->eraseFromParent(); return; } - assert(0 && "Type mismatch."); + dxilutil::EmitErrorOnInstruction( + BCI, "Unsupported cast between struct types with different layouts."); + BCI->replaceAllUsesWith(UndefValue::get(BCI->getType())); + BCI->eraseFromParent(); return; } diff --git a/tools/clang/test/HLSLFileCheck/hlsl/types/cast/similar_layout_structs/cast_as_func_inout_param-strictudt.hlsl b/tools/clang/test/HLSLFileCheck/hlsl/types/cast/similar_layout_structs/cast_as_func_inout_param-strictudt.hlsl new file mode 100644 index 0000000000..0cc094daba --- /dev/null +++ b/tools/clang/test/HLSLFileCheck/hlsl/types/cast/similar_layout_structs/cast_as_func_inout_param-strictudt.hlsl @@ -0,0 +1,40 @@ +// RUN: %dxc -T ps_6_0 -HV 2021 %s | FileCheck %s +// RUN: %dxc -T ps_6_0 -HV 2021 -DSTATIC_GLOBAL %s | FileCheck %s -check-prefix=STATIC_GLOBAL + +// Validate that passing a struct to an inout parameter of a differently laid +// out struct does not hang the compiler. + +// CHECK: call void @dx.op.storeOutput.f32(i32 5, i32 0, i32 0, i8 0, float 1.000000e+00) + +// STATIC_GLOBAL: error: Unsupported cast between struct types with different layouts. + +struct S { + float2 v; +}; + +struct T { + float x, y; +}; + +void f(inout S s) { + s.v.x = 1; +} + +#ifdef STATIC_GLOBAL + +static T g; + +float4 main() : SV_Target { + f((S)g); + return g.x; +} + +#else + +float4 main() : SV_Target { + T t = (T)0; + f((S)t); + return t.x; +} + +#endif diff --git a/tools/clang/test/HLSLFileCheck/hlsl/types/cast/similar_layout_structs/cast_as_func_inout_param.hlsl b/tools/clang/test/HLSLFileCheck/hlsl/types/cast/similar_layout_structs/cast_as_func_inout_param.hlsl new file mode 100644 index 0000000000..66599db9f0 --- /dev/null +++ b/tools/clang/test/HLSLFileCheck/hlsl/types/cast/similar_layout_structs/cast_as_func_inout_param.hlsl @@ -0,0 +1,39 @@ +// RUN: %dxc -T ps_6_0 -HV 2018 %s | FileCheck %s +// RUN: %dxc -T ps_6_0 -HV 2018 -DSTATIC_GLOBAL %s | FileCheck %s -check-prefix=STATIC_GLOBAL + +// Validate that passing a struct to an inout parameter of a differently laid +// out struct does not hang the compiler. + +// CHECK: call void @dx.op.storeOutput.f32(i32 5, i32 0, i32 0, i8 0, float 1.000000e+00) +// STATIC_GLOBAL: error: Unsupported cast between struct types with different layouts. + +struct S { + float2 v; +}; + +struct T { + float x, y; +}; + +void f(inout S s) { + s.v.x = 1; +} + +#ifdef STATIC_GLOBAL + +static T g; + +float4 main() : SV_Target { + f(g); + return g.x; +} + +#else + +float4 main() : SV_Target { + T t = (T)0; + f(t); + return t.x; +} + +#endif