Skip to content

Commit c7e815b

Browse files
isoard-amdAlexandre IsoardIcohedron
authored
[HLSL] Emit lifetime.start before copy-in for inout parameters (#191917)
For inout parameters, Clang was emitting lifetime.start after the copy-in store that initializes the temporary. Per LLVM's lifetime semantics, any access to memory outside its lifetime is undefined behavior, so the copy-in store was technically UB and the value was undefined after lifetime.start. Move EmitLifetimeStart into EmitHLSLOutArgLValues so that it is emitted before EmitInitializationToLValue, putting the copy-in store within the lifetime of the temporary. --------- Co-authored-by: Alexandre Isoard <alexandre.isoard@amd.com> Co-authored-by: Deric C. <cheung.deric@gmail.com>
1 parent 38df8cb commit c7e815b

2 files changed

Lines changed: 96 additions & 2 deletions

File tree

‎clang/lib/CodeGen/CGExpr.cpp‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6360,6 +6360,11 @@ CodeGenFunction::EmitHLSLOutArgLValues(const HLSLOutArgExpr *E, QualType Ty) {
63606360
Address OutTemp = CreateIRTempWithoutCast(ExprTy);
63616361
LValue TempLV = MakeAddrLValue(OutTemp, ExprTy);
63626362

6363+
// Start the lifetime before the copy-in so that the temporary is live when
6364+
// the initial value is written. This ensures the store is within the
6365+
// lifetime and is not killed by a store undef inserted at lifetime.start.
6366+
EmitLifetimeStart(OutTemp.getBasePointer());
6367+
63636368
if (E->isInOut())
63646369
EmitInitializationToLValue(E->getCastedTemporary()->getSourceExpr(),
63656370
TempLV);
@@ -6376,8 +6381,6 @@ LValue CodeGenFunction::EmitHLSLOutArgExpr(const HLSLOutArgExpr *E,
63766381
llvm::Value *Addr = TempLV.getAddress().getBasePointer();
63776382
llvm::Type *ElTy = ConvertTypeForMem(TempLV.getType());
63786383

6379-
EmitLifetimeStart(Addr);
6380-
63816384
Address TmpAddr(Addr, ElTy, TempLV.getAlignment());
63826385
Args.addWriteback(BaseLV, TmpAddr, nullptr, E->getWritebackCast());
63836386
Args.add(RValue::get(TmpAddr, *this), Ty);

‎clang/test/CodeGenHLSL/BasicFeatures/OutArgLifetime.hlsl‎

Lines changed: 91 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,91 @@
1+
// NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py UTC_ARGS: --version 6
2+
// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.0-library -O1 -disable-llvm-passes -emit-llvm -finclude-default-header -o - %s | FileCheck %s
3+
4+
// Check that lifetime.start for an inout argument temporary is emitted
5+
// *before* the copy-in store, so that the store is within the lifetime
6+
// and is not treated as undefined behavior.
7+
8+
// CHECK-LABEL: define hidden void @_Z9incrementRi(
9+
// CHECK-SAME: ptr noalias noundef nonnull align 4 dereferenceable(4) [[I:%.*]]) #[[ATTR0:[0-9]+]] {
10+
// CHECK-NEXT: [[ENTRY:.*:]]
11+
// CHECK-NEXT: [[TMP0:%.*]] = call token @llvm.experimental.convergence.entry()
12+
// CHECK-NEXT: [[I_ADDR:%.*]] = alloca ptr, align 4
13+
// CHECK-NEXT: store ptr [[I]], ptr [[I_ADDR]], align 4, !tbaa [[INTPTR_TBAA6:![0-9]+]]
14+
// CHECK-NEXT: [[TMP1:%.*]] = load ptr, ptr [[I_ADDR]], align 4, !tbaa [[INTPTR_TBAA6]], !nonnull [[META9:![0-9]+]], !align [[META10:![0-9]+]]
15+
// CHECK-NEXT: [[TMP2:%.*]] = load i32, ptr [[TMP1]], align 4, !tbaa [[INT_TBAA2:![0-9]+]]
16+
// CHECK-NEXT: [[ADD:%.*]] = add nsw i32 [[TMP2]], 1
17+
// CHECK-NEXT: store i32 [[ADD]], ptr [[TMP1]], align 4, !tbaa [[INT_TBAA2]]
18+
// CHECK-NEXT: ret void
19+
//
20+
void increment(inout int I) { I += 1; }
21+
22+
// CHECK-LABEL: define hidden void @_Z5resetRi(
23+
// CHECK-SAME: ptr noalias noundef nonnull align 4 dereferenceable(4) [[I:%.*]]) #[[ATTR0]] {
24+
// CHECK-NEXT: [[ENTRY:.*:]]
25+
// CHECK-NEXT: [[TMP0:%.*]] = call token @llvm.experimental.convergence.entry()
26+
// CHECK-NEXT: [[I_ADDR:%.*]] = alloca ptr, align 4
27+
// CHECK-NEXT: store ptr [[I]], ptr [[I_ADDR]], align 4, !tbaa [[INTPTR_TBAA6]]
28+
// CHECK-NEXT: [[TMP1:%.*]] = load ptr, ptr [[I_ADDR]], align 4, !tbaa [[INTPTR_TBAA6]], !nonnull [[META9]], !align [[META10]]
29+
// CHECK-NEXT: store i32 0, ptr [[TMP1]], align 4, !tbaa [[INT_TBAA2]]
30+
// CHECK-NEXT: ret void
31+
//
32+
void reset(out int I) { I = 0; }
33+
34+
// The lifetime.start must come before the copy-in load/store sequence.
35+
// CHECK-LABEL: define hidden noundef i32 @_Z10inout_testi(
36+
// CHECK-SAME: i32 noundef [[X:%.*]]) #[[ATTR0]] {
37+
// CHECK-NEXT: [[ENTRY:.*:]]
38+
// CHECK-NEXT: [[TMP0:%.*]] = call token @llvm.experimental.convergence.entry()
39+
// CHECK-NEXT: [[X_ADDR:%.*]] = alloca i32, align 4
40+
// CHECK-NEXT: [[TMP:%.*]] = alloca i32, align 4
41+
// CHECK-NEXT: store i32 [[X]], ptr [[X_ADDR]], align 4, !tbaa [[INT_TBAA2]]
42+
// CHECK-NEXT: call void @llvm.lifetime.start.p0(ptr [[TMP]]) #[[ATTR3:[0-9]+]]
43+
// CHECK-NEXT: [[TMP1:%.*]] = load i32, ptr [[X_ADDR]], align 4, !tbaa [[INT_TBAA2]]
44+
// CHECK-NEXT: store i32 [[TMP1]], ptr [[TMP]], align 4, !tbaa [[INT_TBAA2]]
45+
// CHECK-NEXT: call void @_Z9incrementRi(ptr noalias noundef nonnull align 4 dereferenceable(4) [[TMP]]) #[[ATTR4:[0-9]+]] [ "convergencectrl"(token [[TMP0]]) ]
46+
// CHECK-NEXT: [[TMP2:%.*]] = load i32, ptr [[TMP]], align 4, !tbaa [[INT_TBAA2]]
47+
// CHECK-NEXT: store i32 [[TMP2]], ptr [[X_ADDR]], align 4, !tbaa [[INT_TBAA2]]
48+
// CHECK-NEXT: call void @llvm.lifetime.end.p0(ptr [[TMP]]) #[[ATTR3]]
49+
// CHECK-NEXT: [[TMP3:%.*]] = load i32, ptr [[X_ADDR]], align 4, !tbaa [[INT_TBAA2]]
50+
// CHECK-NEXT: ret i32 [[TMP3]]
51+
//
52+
int inout_test(int X) {
53+
increment(X);
54+
return X;
55+
}
56+
57+
// For `out` parameters there is no copy-in, so lifetime.start just needs
58+
// to appear before the call with no intervening store to the temporary.
59+
// CHECK-LABEL: define hidden noundef i32 @_Z8out_testv(
60+
// CHECK-SAME: ) #[[ATTR0]] {
61+
// CHECK-NEXT: [[ENTRY:.*:]]
62+
// CHECK-NEXT: [[TMP0:%.*]] = call token @llvm.experimental.convergence.entry()
63+
// CHECK-NEXT: [[X:%.*]] = alloca i32, align 4
64+
// CHECK-NEXT: [[TMP:%.*]] = alloca i32, align 4
65+
// CHECK-NEXT: call void @llvm.lifetime.start.p0(ptr [[X]]) #[[ATTR3]]
66+
// CHECK-NEXT: call void @llvm.lifetime.start.p0(ptr [[TMP]]) #[[ATTR3]]
67+
// CHECK-NEXT: call void @_Z5resetRi(ptr noalias noundef nonnull align 4 dereferenceable(4) [[TMP]]) #[[ATTR4]] [ "convergencectrl"(token [[TMP0]]) ]
68+
// CHECK-NEXT: [[TMP1:%.*]] = load i32, ptr [[TMP]], align 4, !tbaa [[INT_TBAA2]]
69+
// CHECK-NEXT: store i32 [[TMP1]], ptr [[X]], align 4, !tbaa [[INT_TBAA2]]
70+
// CHECK-NEXT: call void @llvm.lifetime.end.p0(ptr [[TMP]]) #[[ATTR3]]
71+
// CHECK-NEXT: [[TMP2:%.*]] = load i32, ptr [[X]], align 4, !tbaa [[INT_TBAA2]]
72+
// CHECK-NEXT: call void @llvm.lifetime.end.p0(ptr [[X]]) #[[ATTR3]]
73+
// CHECK-NEXT: ret i32 [[TMP2]]
74+
//
75+
int out_test() {
76+
int X;
77+
reset(X);
78+
return X;
79+
}
80+
81+
//.
82+
// CHECK: [[INT_TBAA2]] = !{[[META3:![0-9]+]], [[META3]], i64 0}
83+
// CHECK: [[META3]] = !{!"int", [[META4:![0-9]+]], i64 0}
84+
// CHECK: [[META4]] = !{!"omnipotent char", [[META5:![0-9]+]], i64 0}
85+
// CHECK: [[META5]] = !{!"Simple C++ TBAA"}
86+
// CHECK: [[INTPTR_TBAA6]] = !{[[META7:![0-9]+]], [[META7]], i64 0}
87+
// CHECK: [[META7]] = !{!"p1 int", [[META8:![0-9]+]], i64 0}
88+
// CHECK: [[META8]] = !{!"any pointer", [[META4]], i64 0}
89+
// CHECK: [[META9]] = !{}
90+
// CHECK: [[META10]] = !{i64 4}
91+
//.

0 commit comments

Comments
 (0)