Skip to content

[HLSL] get inout/out ABI for array parameters working #111047

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.

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 9 commits into from
Dec 4, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 2 additions & 0 deletions clang/include/clang/AST/Type.h
Original file line number Diff line number Diff line change
Expand Up @@ -3754,6 +3754,8 @@ class ArrayParameterType : public ConstantArrayType {
static bool classof(const Type *T) {
return T->getTypeClass() == ArrayParameter;
}

QualType getConstantArrayType(const ASTContext &Ctx) const;
};

/// Represents a C array with an unspecified size. For example 'int A[]' has
Expand Down
6 changes: 6 additions & 0 deletions clang/lib/AST/Type.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -267,6 +267,12 @@ void ConstantArrayType::Profile(llvm::FoldingSetNodeID &ID,
SizeExpr->Profile(ID, Context, true);
}

QualType ArrayParameterType::getConstantArrayType(const ASTContext &Ctx) const {
return Ctx.getConstantArrayType(getElementType(), getSize(), getSizeExpr(),
getSizeModifier(),
getIndexTypeQualifiers().getAsOpaqueValue());
}

DependentSizedArrayType::DependentSizedArrayType(QualType et, QualType can,
Expr *e, ArraySizeModifier sm,
unsigned tq,
Expand Down
16 changes: 13 additions & 3 deletions clang/lib/CodeGen/CGCall.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4725,15 +4725,17 @@ void CodeGenFunction::EmitCallArg(CallArgList &args, const Expr *E,
return emitWritebackArg(*this, args, CRE);
}

assert(type->isReferenceType() == E->isGLValue() &&
"reference binding to unmaterialized r-value!");

// Add writeback for HLSLOutParamExpr.
// Needs to be before the assert below because HLSLOutArgExpr is an LValue
// and is not a reference.
if (const HLSLOutArgExpr *OE = dyn_cast<HLSLOutArgExpr>(E)) {
EmitHLSLOutArgExpr(OE, args, type);
return;
}

assert(type->isReferenceType() == E->isGLValue() &&
"reference binding to unmaterialized r-value!");

if (E->isGLValue()) {
assert(E->getObjectKind() == OK_Ordinary);
return args.add(EmitReferenceBindingToExpr(E), type);
Expand Down Expand Up @@ -5322,6 +5324,14 @@ RValue CodeGenFunction::EmitCall(const CGFunctionInfo &CallInfo,
IRCallArgs[FirstIRArg] = Val;
break;
}
} else if (I->getType()->isArrayParameterType()) {
// Don't produce a temporary for ArrayParameterType arguments.
// ArrayParameterType arguments are only created from
// HLSL_ArrayRValue casts and HLSLOutArgExpr expressions, both
// of which create temporaries already. This allows us to just use the
// scalar for the decayed array pointer as the argument directly.
IRCallArgs[FirstIRArg] = I->getKnownRValue().getScalarVal();
break;
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think this might be the wrong solution. If we instead make the parameter ABI Direct, it should end up doing effectively what you're accomplishing here.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Bleh... I spent some time today looking at this, and I think I've convinced myself that this is probably the simplest approach.

Without this you get a redundant copy because the cast that produces the ArrayParameterType. I have one small request to change the comment above.

}

// For non-aggregate args and aggregate args meeting conditions above
Expand Down
7 changes: 5 additions & 2 deletions clang/lib/CodeGen/CGExpr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5827,9 +5827,12 @@ LValue CodeGenFunction::EmitBinaryOperatorLValue(const BinaryOperator *E) {
// This function implements trivial copy assignment for HLSL's
// assignable constant arrays.
LValue CodeGenFunction::EmitHLSLArrayAssignLValue(const BinaryOperator *E) {
LValue TrivialAssignmentRHS = EmitLValue(E->getRHS());
// Don't emit an LValue for the RHS because it might not be an LValue
LValue LHS = EmitLValue(E->getLHS());
EmitAggregateAssign(LHS, TrivialAssignmentRHS, E->getLHS()->getType());
// In C the RHS of an assignment operator is an RValue.
// EmitAggregateAssign takes anan LValue for the RHS. Instead we can call
// EmitInitializationToLValue to emit an RValue into an LValue.
EmitInitializationToLValue(E->getRHS(), LHS);
return LHS;
}

Expand Down
9 changes: 9 additions & 0 deletions clang/lib/Sema/Sema.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -723,6 +723,15 @@ ExprResult Sema::ImpCastExprToType(Expr *E, QualType Ty,
QualType ExprTy = Context.getCanonicalType(E->getType());
QualType TypeTy = Context.getCanonicalType(Ty);

// This cast is used in place of a regular LValue to RValue cast for
// HLSL Array Parameter Types. It needs to be emitted even if
// ExprTy == TypeTy, except if E is an HLSLOutArgExpr
// Emitting a cast in that case will prevent HLSLOutArgExpr from
// being handled properly in EmitCallArg
if (Kind == CK_HLSLArrayRValue && !isa<HLSLOutArgExpr>(E))
return ImplicitCastExpr::Create(Context, Ty, Kind, E, BasePath, VK,
CurFPFeatureOverrides());

if (ExprTy == TypeTy)
return E;

Expand Down
19 changes: 15 additions & 4 deletions clang/lib/Sema/SemaExprCXX.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4431,10 +4431,21 @@ Sema::PerformImplicitConversion(Expr *From, QualType ToType,
break;

case ICK_HLSL_Array_RValue:
FromType = Context.getArrayParameterType(FromType);
From = ImpCastExprToType(From, FromType, CK_HLSLArrayRValue, VK_PRValue,
/*BasePath=*/nullptr, CCK)
.get();
if (ToType->isArrayParameterType()) {
FromType = Context.getArrayParameterType(FromType);
From = ImpCastExprToType(From, FromType, CK_HLSLArrayRValue, VK_PRValue,
/*BasePath=*/nullptr, CCK)
.get();
} else { // FromType must be ArrayParameterType
assert(FromType->isArrayParameterType() &&
"FromType must be ArrayParameterType in ICK_HLSL_Array_RValue \
if it is not ToType");
const ArrayParameterType *APT = cast<ArrayParameterType>(FromType);
FromType = APT->getConstantArrayType(Context);
From = ImpCastExprToType(From, FromType, CK_HLSLArrayRValue, VK_PRValue,
/*BasePath=*/nullptr, CCK)
.get();
}
break;

case ICK_Function_To_Pointer:
Expand Down
50 changes: 30 additions & 20 deletions clang/lib/Sema/SemaOverload.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2236,33 +2236,24 @@ static bool IsStandardConversion(Sema &S, Expr* From, QualType ToType,
return false;
}
}
// Lvalue-to-rvalue conversion (C++11 4.1):
// A glvalue (3.10) of a non-function, non-array type T can
// be converted to a prvalue.
bool argIsLValue = From->isGLValue();
if (argIsLValue && !FromType->canDecayToPointerType() &&
S.Context.getCanonicalType(FromType) != S.Context.OverloadTy) {
SCS.First = ICK_Lvalue_To_Rvalue;

// C11 6.3.2.1p2:
// ... if the lvalue has atomic type, the value has the non-atomic version
// of the type of the lvalue ...
if (const AtomicType *Atomic = FromType->getAs<AtomicType>())
FromType = Atomic->getValueType();

// If T is a non-class type, the type of the rvalue is the
// cv-unqualified version of T. Otherwise, the type of the rvalue
// is T (C++ 4.1p1). C++ can't get here with class types; in C, we
// just strip the qualifiers because they don't matter.
FromType = FromType.getUnqualifiedType();
} else if (S.getLangOpts().HLSL && FromType->isConstantArrayType() &&
ToType->isConstantArrayType()) {
bool argIsLValue = From->isGLValue();
// To handle conversion from ArrayParameterType to ConstantArrayType
// this block must be above the one below because Array parameters
// do not decay and when handling HLSLOutArgExprs and
// the From expression is an LValue.
if (S.getLangOpts().HLSL && FromType->isConstantArrayType() &&
ToType->isConstantArrayType()) {
// HLSL constant array parameters do not decay, so if the argument is a
// constant array and the parameter is an ArrayParameterType we have special
// handling here.
if (ToType->isArrayParameterType()) {
FromType = S.Context.getArrayParameterType(FromType);
SCS.First = ICK_HLSL_Array_RValue;
} else if (FromType->isArrayParameterType()) {
const ArrayParameterType *APT = cast<ArrayParameterType>(FromType);
FromType = APT->getConstantArrayType(S.Context);
SCS.First = ICK_HLSL_Array_RValue;
} else {
SCS.First = ICK_Identity;
}
Expand All @@ -2273,6 +2264,25 @@ static bool IsStandardConversion(Sema &S, Expr* From, QualType ToType,

SCS.setAllToTypes(ToType);
return true;
} else if (argIsLValue && !FromType->canDecayToPointerType() &&
S.Context.getCanonicalType(FromType) != S.Context.OverloadTy) {
// Lvalue-to-rvalue conversion (C++11 4.1):
// A glvalue (3.10) of a non-function, non-array type T can
// be converted to a prvalue.

SCS.First = ICK_Lvalue_To_Rvalue;

// C11 6.3.2.1p2:
// ... if the lvalue has atomic type, the value has the non-atomic version
// of the type of the lvalue ...
if (const AtomicType *Atomic = FromType->getAs<AtomicType>())
FromType = Atomic->getValueType();

// If T is a non-class type, the type of the rvalue is the
// cv-unqualified version of T. Otherwise, the type of the rvalue
// is T (C++ 4.1p1). C++ can't get here with class types; in C, we
// just strip the qualifiers because they don't matter.
FromType = FromType.getUnqualifiedType();
} else if (FromType->isArrayType()) {
// Array-to-pointer conversion (C++ 4.2)
SCS.First = ICK_Array_To_Pointer;
Expand Down
4 changes: 3 additions & 1 deletion clang/lib/Sema/SemaType.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5681,6 +5681,9 @@ static TypeSourceInfo *GetFullTypeForDeclarator(TypeProcessingState &state,
assert(!T.isNull() && "T must not be null at the end of this function");
if (!AreDeclaratorChunksValid)
return Context.getTrivialTypeSourceInfo(T);

if (state.didParseHLSLParamMod() && !T->isConstantArrayType())
T = S.HLSL().getInoutParameterType(T);
return GetTypeSourceInfoForDeclarator(state, T, TInfo);
}

Expand Down Expand Up @@ -8634,7 +8637,6 @@ static void HandleHLSLParamModifierAttr(TypeProcessingState &State,
return;
if (Attr.getSemanticSpelling() == HLSLParamModifierAttr::Keyword_inout ||
Attr.getSemanticSpelling() == HLSLParamModifierAttr::Keyword_out) {
CurType = S.HLSL().getInoutParameterType(CurType);
State.setParsedHLSLParamMod(true);
}
}
Expand Down
63 changes: 63 additions & 0 deletions clang/test/AST/HLSL/ArrayOutArgExpr.hlsl
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
// RUN: %clang_cc1 -triple dxil-pc-shadermodel6.0-library -x hlsl -ast-dump %s | FileCheck %s

// CHECK-LABEL: increment
void increment(inout int Arr[2]) {
for (int I = 0; I < 2; I++)
Arr[0] += 2;
}

// CHECK-LABEL: call
// CHECK: CallExpr 0x{{.*}} {{.*}} 'void'
// CHECK: ImplicitCastExpr 0x{{.*}} {{.*}} 'void (*)(inout int[2])' <FunctionToPointerDecay>
// CHECK: DeclRefExpr 0x{{.*}} {{.*}} 'void (inout int[2])' lvalue Function 0x{{.*}} 'increment' 'void (inout int[2])'
// CHECK: HLSLOutArgExpr 0x{{.*}} {{.*}} 'int[2]' lvalue inout
// CHECK: OpaqueValueExpr [[A:0x.*]] {{.*}} 'int[2]' lvalue
// CHECK: DeclRefExpr [[B:0x.*]] {{.*}} 'int[2]' lvalue Var [[E:0x.*]] 'A' 'int[2]'
// CHECK: OpaqueValueExpr [[C:0x.*]] {{.*}} 'int[2]' lvalue
// CHECK: ImplicitCastExpr [[D:0x.*]] {{.*}} 'int[2]' <HLSLArrayRValue>
// CHECK: OpaqueValueExpr [[A]] {{.*}} 'int[2]' lvalue
// CHECK: DeclRefExpr [[B]] {{.*}} 'int[2]' lvalue Var [[E]] 'A' 'int[2]'
// CHECK: BinaryOperator 0x{{.*}} {{.*}} 'int[2]' lvalue '='
// CHECK: OpaqueValueExpr [[A]] {{.*}} 'int[2]' lvalue
// CHECK: DeclRefExpr 0x{{.*}} {{.*}} 'int[2]' lvalue Var [[E]] 'A' 'int[2]'
// CHECK: ImplicitCastExpr 0x{{.*}} {{.*}} 'int[2]' <HLSLArrayRValue>
// CHECK: OpaqueValueExpr [[C]] {{.*}} 'int[2]' lvalue
// CHECK: ImplicitCastExpr [[D]] {{.*}} 'int[2]' <HLSLArrayRValue>
// CHECK: OpaqueValueExpr [[A]] {{.*}} 'int[2]' lvalue
// CHECK: DeclRefExpr [[B]] {{.*}} 'int[2]' lvalue Var [[E]] 'A' 'int[2]'
export int call() {
int A[2] = { 0, 1 };
increment(A);
return A[0];
}

// CHECK-LABEL: fn2
void fn2(out int Arr[2]) {
Arr[0] += 5;
Arr[1] += 6;
}

// CHECK-LABEL: call2
// CHECK: CallExpr 0x{{.*}} {{.*}} 'void'
// CHECK: ImplicitCastExpr 0x{{.*}} {{.*}} 'void (*)(out int[2])' <FunctionToPointerDecay>
// CHECK: DeclRefExpr 0x{{.*}} {{.*}} 'void (out int[2])' lvalue Function 0x{{.*}} 'fn2' 'void (out int[2])'
// CHECK: HLSLOutArgExpr 0x{{.*}} {{.*}} 'int[2]' lvalue out
// CHECK: OpaqueValueExpr [[A:0x.*]] {{.*}} 'int[2]' lvalue
// CHECK: DeclRefExpr [[B:0x.*]] {{.*}} 'int[2]' lvalue Var [[E:0x.*]] 'A' 'int[2]'
// CHECK: OpaqueValueExpr [[C:0x.*]] {{.*}} 'int[2]' lvalue
// CHECK: ImplicitCastExpr [[D:0x.*]] {{.*}} 'int[2]' <HLSLArrayRValue>
// CHECK: OpaqueValueExpr [[A]] {{.*}} 'int[2]' lvalue
// CHECK: DeclRefExpr [[B]] {{.*}} 'int[2]' lvalue Var [[E]] 'A' 'int[2]'
// CHECK: BinaryOperator 0x{{.*}} {{.*}} 'int[2]' lvalue '='
// CHECK: OpaqueValueExpr [[A]] {{.*}} 'int[2]' lvalue
// CHECK: DeclRefExpr [[B]] {{.*}} 'int[2]' lvalue Var [[E]] 'A' 'int[2]'
// CHECK: ImplicitCastExpr 0x{{.*}} {{.*}} 'int[2]' <HLSLArrayRValue>
// CHECK: OpaqueValueExpr [[C]] {{.*}} 'int[2]' lvalue
// CHECK: ImplicitCastExpr [[D]] {{.*}} 'int[2]' <HLSLArrayRValue>
// CHECK: OpaqueValueExpr [[A]] {{.*}} 'int[2]' lvalue
// CHECK: DeclRefExpr [[B]] {{.*}} 'int[2]' lvalue Var [[E]] 'A' 'int[2]'
export int call2() {
int A[2] = { 0, 1 };
fn2(A);
return 1;
}
20 changes: 9 additions & 11 deletions clang/test/CodeGenHLSL/ArrayAssignable.hlsl
Original file line number Diff line number Diff line change
Expand Up @@ -100,18 +100,16 @@ void arr_assign6() {
}

// CHECK-LABEL: define void {{.*}}arr_assign7
// CHECK: [[Arr3:%.*]] = alloca [2 x [2 x i32]], align 4
// CHECK-NEXT: [[Arr4:%.*]] = alloca [2 x [2 x i32]], align 4
// CHECK-NEXT: [[Tmp:%.*]] = alloca [2 x i32], align 4
// CHECK: [[Arr:%.*]] = alloca [2 x [2 x i32]], align 4
// CHECK-NEXT: [[Arr2:%.*]] = alloca [2 x [2 x i32]], align 4
// CHECK-NOT: alloca
// CHECK-NEXT: call void @llvm.memcpy.p0.p0.i32(ptr align 4 [[Arr3]], ptr align 4 {{@.*}}, i32 16, i1 false)
// CHECK-NEXT: call void @llvm.memcpy.p0.p0.i32(ptr align 4 [[Arr4]], ptr align 4 {{@.*}}, i32 16, i1 false)
// CHECK-NEXT: store i32 6, ptr [[Tmp]], align 4
// CHECK-NEXT: [[AIE:%.*]] = getelementptr inbounds i32, ptr [[Tmp]], i32 1
// CHECK-NEXT: store i32 6, ptr [[AIE]], align 4
// CHECK-NEXT: call void @llvm.memcpy.p0.p0.i32(ptr align 4 [[Arr3]], ptr align 4 [[Arr4]], i32 16, i1 false)
// CHECK-NEXT: [[Idx:%.*]] = getelementptr inbounds [2 x [2 x i32]], ptr [[Arr3]], i32 0, i32 0
// CHECK-NEXT: call void @llvm.memcpy.p0.p0.i32(ptr align 4 [[Idx]], ptr align 4 [[Tmp]], i32 8, i1 false)
// CHECK-NEXT: call void @llvm.memcpy.p0.p0.i32(ptr align 4 [[Arr]], ptr align 4 {{@.*}}, i32 16, i1 false)
// CHECK-NEXT: call void @llvm.memcpy.p0.p0.i32(ptr align 4 [[Arr2]], ptr align 4 {{@.*}}, i32 16, i1 false)
// CHECK-NEXT: call void @llvm.memcpy.p0.p0.i32(ptr align 4 [[Arr]], ptr align 4 [[Arr2]], i32 16, i1 false)
// CHECK-NEXT: [[Idx:%.*]] = getelementptr inbounds [2 x [2 x i32]], ptr [[Arr]], i32 0, i32 0
// CHECK-NEXT: store i32 6, ptr [[Idx]], align 4
// CHECK-NEXT: [[Idx2:%.*]] = getelementptr inbounds i32, ptr %arrayidx, i32 1
// CHECK-NEXT: store i32 6, ptr [[Idx2]], align 4
// CHECK-NEXT: ret void
void arr_assign7() {
int Arr[2][2] = {{0, 1}, {2, 3}};
Expand Down
Loading
Loading