Skip to content

[HLSL] Avoid putting the byval attribute on out and inout parameters #150495

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Jul 24, 2025
Merged

[HLSL] Avoid putting the byval attribute on out and inout parameters #150495

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
16b7231
Avoid putting ByVal on out and inout parameters
Icohedron Jul 24, 2025
5791160
Apply clang-format
Icohedron Jul 24, 2025
155e511
Revise comment to be more clear about why ByVal and DeadOnReturn must…
Icohedron Jul 24, 2025
dc379c7
Small addendum to the comment
Icohedron Jul 24, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 22 additions & 14 deletions clang/lib/CodeGen/CGCall.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2852,20 +2852,28 @@ void CodeGenModule::ConstructAttributeList(StringRef Name,
if (AI.getInReg())
Attrs.addAttribute(llvm::Attribute::InReg);

// Depending on the ABI, this may be either a byval or a dead_on_return
// argument.
if (AI.getIndirectByVal()) {
Attrs.addByValAttr(getTypes().ConvertTypeForMem(ParamType));
} else {
// Add dead_on_return when the object's lifetime ends in the callee.
// This includes trivially-destructible objects, as well as objects
// whose destruction / clean-up is carried out within the callee (e.g.,
// Obj-C ARC-managed structs, MSVC callee-destroyed objects).
if (!ParamType.isDestructedType() || !ParamType->isRecordType() ||
ParamType->castAs<RecordType>()
->getDecl()
->isParamDestroyedInCallee())
Attrs.addAttribute(llvm::Attribute::DeadOnReturn);
// HLSL out and inout parameters must not be marked with ByVal or
Copy link
Contributor

@spall spall Jul 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe expand this comment to explain why it isn't byval
"HLSL out/inout parameters are not ByVal because the caller is meant to see the callees changes"

// DeadOnReturn attributes, as they would be eliminated by Dead Store
// Elimination.
if (auto ParamABI = FI.getExtParameterInfo(ArgNo).getABI();
ParamABI != ParameterABI::HLSLOut &&
ParamABI != ParameterABI::HLSLInOut) {

// Depending on the ABI, this may be either a byval or a dead_on_return
// argument.
if (AI.getIndirectByVal()) {
Attrs.addByValAttr(getTypes().ConvertTypeForMem(ParamType));
} else {
// Add dead_on_return when the object's lifetime ends in the callee.
// This includes trivially-destructible objects, as well as objects
// whose destruction / clean-up is carried out within the callee
// (e.g., Obj-C ARC-managed structs, MSVC callee-destroyed objects).
if (!ParamType.isDestructedType() || !ParamType->isRecordType() ||
ParamType->castAs<RecordType>()
->getDecl()
->isParamDestroyedInCallee())
Attrs.addAttribute(llvm::Attribute::DeadOnReturn);
}
}

auto *Decl = ParamType->getAsRecordDecl();
Expand Down
12 changes: 6 additions & 6 deletions clang/test/CodeGenHLSL/BasicFeatures/ArrayOutputArguments.hlsl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ void increment(inout int Arr[2]) {
// CHECK-NEXT: [[Tmp:%.*]] = alloca [2 x i32], align 4
// CHECK-NEXT: call void @llvm.memcpy.p0.p0.i32(ptr align 4 [[A]], ptr align 4 @{{.*}}, i32 8, i1 false)
// CHECK-NEXT: call void @llvm.memcpy.p0.p0.i32(ptr align 4 [[Tmp]], ptr align 4 [[A]], i32 8, i1 false)
// CHECK-NEXT: call void @{{.*}}increment{{.*}}(ptr noalias noundef byval([2 x i32]) align 4 [[Tmp]])
// CHECK-NEXT: call void @{{.*}}increment{{.*}}(ptr noalias noundef align 4 [[Tmp]])
// CHECK-NEXT: call void @llvm.memcpy.p0.p0.i32(ptr align 4 [[A]], ptr align 4 [[Tmp]], i32 8, i1 false)
// CHECK-NEXT: [[Idx:%.*]] = getelementptr inbounds [2 x i32], ptr [[A]], i32 0, i32 0
// CHECK-NEXT: [[B:%.*]] = load i32, ptr [[Idx]], align 4
Expand All @@ -32,7 +32,7 @@ void fn2(out int Arr[2]) {
// CHECK: [[A:%.*]] = alloca [2 x i32], align 4
// CHECK-NEXT: [[Tmp:%.*]] = alloca [2 x i32], align 4
// CHECK-NEXT: call void @llvm.memcpy.p0.p0.i32(ptr align 4 [[A]], ptr align 4 @{{.*}}, i32 8, i1 false)
// CHECK-NEXT: call void @{{.*}}fn2{{.*}}(ptr noalias noundef byval([2 x i32]) align 4 [[Tmp]])
// CHECK-NEXT: call void @{{.*}}fn2{{.*}}(ptr noalias noundef align 4 [[Tmp]])
// CHECK-NEXT: call void @llvm.memcpy.p0.p0.i32(ptr align 4 [[A]], ptr align 4 [[Tmp]], i32 8, i1 false)
// CHECK-NEXT: [[Idx:%.*]] = getelementptr inbounds [2 x i32], ptr [[A]], i32 0, i32 0
// CHECK-NEXT: [[B:%.*]] = load i32, ptr [[Idx]], align 4
Expand All @@ -56,7 +56,7 @@ void nestedCall(inout int Arr[2], uint index) {
// CHECK-NEXT: [[Tmp:%.*]] = alloca [2 x i32], align 4
// CHECK-NEXT: call void @llvm.memcpy.p0.p0.i32(ptr align 4 [[A]], ptr align 4 @{{.*}}, i32 8, i1 false)
// CHECK-NEXT: call void @llvm.memcpy.p0.p0.i32(ptr align 4 [[Tmp]], ptr align 4 [[A]], i32 8, i1 false)
// CHECK-NEXT: call void @{{.*}}nestedCall{{.*}}(ptr noalias noundef byval([2 x i32]) align 4 [[Tmp]], i32 noundef 0)
// CHECK-NEXT: call void @{{.*}}nestedCall{{.*}}(ptr noalias noundef align 4 [[Tmp]], i32 noundef 0)
// CHECK-NEXT: call void @llvm.memcpy.p0.p0.i32(ptr align 4 [[A]], ptr align 4 [[Tmp]], i32 8, i1 false)
// CHECK-NEXT: [[Idx:%.*]] = getelementptr inbounds [2 x i32], ptr [[A]], i32 0, i32 1
// CHECK-NEXT: [[B:%.*]] = load i32, ptr [[Idx]], align 4
Expand All @@ -70,7 +70,7 @@ export int arrayCall3() {
// CHECK-LABEL: outerCall
// CHECK: [[Tmp:%.*]] = alloca [2 x i32], align 4
// CHECK-NEXT: call void @llvm.memcpy.p0.p0.i32(ptr align 4 [[Tmp]], ptr align 4 %{{.*}}, i32 8, i1 false)
// CHECK-NEXT: call void {{.*}}increment{{.*}}(ptr noalias noundef byval([2 x i32]) align 4 [[Tmp]])
// CHECK-NEXT: call void {{.*}}increment{{.*}}(ptr noalias noundef align 4 [[Tmp]])
// CHECK-NEXT: call void @llvm.memcpy.p0.p0.i32(ptr align 4 {{.*}}, ptr align 4 [[Tmp]], i32 8, i1 false)
// CHECK-NEXT: ret void
void outerCall(inout int Arr[2]) {
Expand All @@ -82,7 +82,7 @@ void outerCall(inout int Arr[2]) {
// CHECK-NEXT: [[Tmp:%.*]] = alloca [2 x i32], align 4
// CHECK-NEXT: call void @llvm.memcpy.p0.p0.i32(ptr align 4 [[A]], ptr align 4 @{{.*}}, i32 8, i1 false)
// CHECK-NEXT: call void @llvm.memcpy.p0.p0.i32(ptr align 4 [[Tmp]], ptr align 4 [[A]], i32 8, i1 false)
// CHECK-NEXT: call void @{{.*}}outerCall{{.*}}(ptr noalias noundef byval([2 x i32]) align 4 [[Tmp]])
// CHECK-NEXT: call void @{{.*}}outerCall{{.*}}(ptr noalias noundef align 4 [[Tmp]])
// CHECK-NEXT: call void @llvm.memcpy.p0.p0.i32(ptr align 4 [[A]], ptr align 4 [[Tmp]], i32 8, i1 false)
// CHECK-NEXT: [[Idx:%.*]] = getelementptr inbounds [2 x i32], ptr [[A]], i32 0, i32 0
// CHECK-NEXT: [[B:%.*]] = load i32, ptr [[Idx]], align 4
Expand Down Expand Up @@ -110,7 +110,7 @@ void outerCall2(inout int Arr[2]) {
// CHECK-NEXT: [[Tmp:%.*]] = alloca [2 x i32], align 4
// CHECK-NEXT: call void @llvm.memcpy.p0.p0.i32(ptr align 4 [[A]], ptr align 4 @{{.*}}, i32 8, i1 false)
// CHECK-NEXT: call void @llvm.memcpy.p0.p0.i32(ptr align 4 [[Tmp]], ptr align 4 [[A]], i32 8, i1 false)
// CHECK-NEXT: call void @{{.*}}outerCall2{{.*}}(ptr noalias noundef byval([2 x i32]) align 4 [[Tmp]])
// CHECK-NEXT: call void @{{.*}}outerCall2{{.*}}(ptr noalias noundef align 4 [[Tmp]])
// CHECK-NEXT: call void @llvm.memcpy.p0.p0.i32(ptr align 4 [[A]], ptr align 4 [[Tmp]], i32 8, i1 false)
// CHECK-NEXT: [[Idx:%.*]] = getelementptr inbounds [2 x i32], ptr [[A]], i32 0, i32 0
// CHECK-NEXT: [[B:%.*]] = load i32, ptr [[Idx]], align 4
Expand Down