Skip to content

Commit b65b4ed

Browse files
spallspall
committed
addressing pr comments
1 parent cd022bb commit b65b4ed

File tree

12 files changed

+140
-36
lines changed

12 files changed

+140
-36
lines changed

‎clang/include/clang/AST/ASTContext.h‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1480,10 +1480,6 @@ class ASTContext : public RefCountedBase<ASTContext> {
14801480
/// type to the decayed type.
14811481
QualType getDecayedType(QualType Orig, QualType Decayed) const;
14821482

1483-
/// Return the uniqued reference to a constant array type from the
1484-
/// original array parameter type.
1485-
QualType getConstantArrayFromArrayParameterType(QualType Ty) const;
1486-
14871483
/// Return the uniqued reference to a specified array parameter type from the
14881484
/// original array type.
14891485
QualType getArrayParameterType(QualType Ty) const;

‎clang/include/clang/AST/Type.h‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3770,6 +3770,8 @@ class ArrayParameterType : public ConstantArrayType {
37703770
static bool classof(const Type *T) {
37713771
return T->getTypeClass() == ArrayParameter;
37723772
}
3773+
3774+
QualType getConstantArrayType(const ASTContext &Ctx) const;
37733775
};
37743776

37753777
/// Represents a C array with an unspecified size. For example 'int A[]' has

‎clang/lib/AST/ASTContext.cpp‎

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -3852,16 +3852,6 @@ QualType ASTContext::getDecayedType(QualType T) const {
38523852
return getDecayedType(T, Decayed);
38533853
}
38543854

3855-
QualType ASTContext::getConstantArrayFromArrayParameterType(QualType Ty) const {
3856-
if (Ty->isConstantArrayType() && !Ty->isArrayParameterType())
3857-
return Ty;
3858-
assert(Ty->isArrayParameterType() && "Ty must be an array parameter type.");
3859-
const auto *ATy = cast<ArrayParameterType>(Ty);
3860-
return getConstantArrayType(ATy->getElementType(), ATy->getSize(),
3861-
ATy->getSizeExpr(), ATy->getSizeModifier(),
3862-
ATy->getIndexTypeQualifiers().getAsOpaqueValue());
3863-
}
3864-
38653855
QualType ASTContext::getArrayParameterType(QualType Ty) const {
38663856
if (Ty->isArrayParameterType())
38673857
return Ty;

‎clang/lib/AST/Type.cpp‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -241,6 +241,12 @@ void ConstantArrayType::Profile(llvm::FoldingSetNodeID &ID,
241241
SizeExpr->Profile(ID, Context, true);
242242
}
243243

244+
QualType ArrayParameterType::getConstantArrayType(const ASTContext &Ctx) const {
245+
return Ctx.getConstantArrayType(getElementType(), getSize(), getSizeExpr(),
246+
getSizeModifier(),
247+
getIndexTypeQualifiers().getAsOpaqueValue());
248+
}
249+
244250
DependentSizedArrayType::DependentSizedArrayType(QualType et, QualType can,
245251
Expr *e, ArraySizeModifier sm,
246252
unsigned tq,

‎clang/lib/CodeGen/CGCall.cpp‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4726,16 +4726,17 @@ void CodeGenFunction::EmitCallArg(CallArgList &args, const Expr *E,
47264726
return emitWritebackArg(*this, args, CRE);
47274727
}
47284728

4729-
assert(type->isArrayParameterType() ||
4730-
(type->isReferenceType() == E->isGLValue()) &&
4731-
"reference binding to unmaterialized r-value!");
4732-
47334729
// Add writeback for HLSLOutParamExpr.
4730+
// Needs to be before the assert below because HLSLOutArgExpr is an LValue
4731+
// and is not a reference.
47344732
if (const HLSLOutArgExpr *OE = dyn_cast<HLSLOutArgExpr>(E)) {
47354733
EmitHLSLOutArgExpr(OE, args, type);
47364734
return;
47374735
}
47384736

4737+
assert(type->isReferenceType() == E->isGLValue() &&
4738+
"reference binding to unmaterialized r-value!");
4739+
47394740
if (E->isGLValue()) {
47404741
assert(E->getObjectKind() == OK_Ordinary);
47414742
return args.add(EmitReferenceBindingToExpr(E), type);

‎clang/lib/CodeGen/CGExpr.cpp‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5832,9 +5832,9 @@ LValue CodeGenFunction::EmitBinaryOperatorLValue(const BinaryOperator *E) {
58325832
LValue CodeGenFunction::EmitHLSLArrayAssignLValue(const BinaryOperator *E) {
58335833
// Don't emit an LValue for the RHS because it might not be an LValue
58345834
LValue LHS = EmitLValue(E->getLHS());
5835-
// In C assignment operator RHS is often an RValue.
5836-
// EmitAggregateAssign expects an LValue for the RHS so call the below
5837-
// function instead.
5835+
// In C the RHS of an assignment operator is an RValue.
5836+
// EmitAggregateAssign takes anan LValue for the RHS. Instead we can call
5837+
// EmitInitializationToLValue to emit an RValue into an LValue.
58385838
EmitInitializationToLValue(E->getRHS(), LHS);
58395839
return LHS;
58405840
}

‎clang/lib/Sema/Sema.cpp‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -726,6 +726,15 @@ ExprResult Sema::ImpCastExprToType(Expr *E, QualType Ty,
726726
QualType ExprTy = Context.getCanonicalType(E->getType());
727727
QualType TypeTy = Context.getCanonicalType(Ty);
728728

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

‎clang/lib/Sema/SemaExprCXX.cpp‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4437,7 +4437,11 @@ Sema::PerformImplicitConversion(Expr *From, QualType ToType,
44374437
/*BasePath=*/nullptr, CCK)
44384438
.get();
44394439
} else { // FromType must be ArrayParameterType
4440-
FromType = Context.getConstantArrayFromArrayParameterType(FromType);
4440+
assert(FromType->isArrayParameterType() &&
4441+
"FromType must be ArrayParameterType in ICK_HLSL_Array_RValue \
4442+
if it is not ToType");
4443+
const ArrayParameterType *APT = cast<ArrayParameterType>(FromType);
4444+
FromType = APT->getConstantArrayType(Context);
44414445
From = ImpCastExprToType(From, FromType, CK_HLSLArrayRValue, VK_PRValue,
44424446
/*BasePath=*/nullptr, CCK)
44434447
.get();

‎clang/lib/Sema/SemaOverload.cpp‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2244,6 +2244,10 @@ static bool IsStandardConversion(Sema &S, Expr* From, QualType ToType,
22442244
}
22452245

22462246
bool argIsLValue = From->isGLValue();
2247+
// To handle conversion from ArrayParameterType to ConstantArrayType
2248+
// this block must be above the one below because Array parameters
2249+
// do not decay and when handling HLSLOutArgExprs and
2250+
// the From expression is an LValue.
22472251
if (S.getLangOpts().HLSL && FromType->isConstantArrayType() &&
22482252
ToType->isConstantArrayType()) {
22492253
// HLSL constant array parameters do not decay, so if the argument is a
@@ -2253,7 +2257,8 @@ static bool IsStandardConversion(Sema &S, Expr* From, QualType ToType,
22532257
FromType = S.Context.getArrayParameterType(FromType);
22542258
SCS.First = ICK_HLSL_Array_RValue;
22552259
} else if (FromType->isArrayParameterType()) {
2256-
FromType = S.Context.getConstantArrayFromArrayParameterType(FromType);
2260+
const ArrayParameterType *APT = cast<ArrayParameterType>(FromType);
2261+
FromType = APT->getConstantArrayType(S.Context);
22572262
SCS.First = ICK_HLSL_Array_RValue;
22582263
} else {
22592264
SCS.First = ICK_Identity;

‎clang/test/CodeGenHLSL/ArrayTemporary.hlsl‎

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -46,8 +46,9 @@ void call3() {
4646
}
4747

4848
// CHECK-LABEL: define void {{.*}}call4{{.*}}(ptr
49+
// CHECK-SAME: noundef byval([2 x [2 x float]]) align 4 [[Arr:%.*]])
4950
// CHECK: [[Tmp:%.*]] = alloca [2 x [2 x float]]
50-
// CHECK: store ptr {{.*}}, ptr [[Tmp]], align 4
51+
// CHECK: call void @llvm.memcpy.p0.p0.i32(ptr align 4 [[Tmp]], ptr align 4 [[Arr]], i32 16, i1 false)
5152
// CHECK: call void {{.*}}fn3{{.*}}(ptr noundef byval([2 x [2 x float]]) align 4 [[Tmp]])
5253

5354
void call4(float Arr[2][2]) {
@@ -66,12 +67,12 @@ void call4(float Arr[2][2]) {
6667
// CHECK: [[Tmp1:%.*]] = alloca [2 x float]
6768
// CHECK: [[Tmp2:%.*]] = alloca [4 x float]
6869
// CHECK: [[Tmp3:%.*]] = alloca [3 x i32]
69-
// CHECK: store ptr {{.*}}, ptr [[Tmp1]], align 4
70-
// CHECK: call void @"??$template_fn@$$BY01M@@YAXY01M@Z"(ptr noundef byval([2 x float]) align 4 [[Tmp1]])
71-
// CHECK: store ptr {{.*}}, ptr [[Tmp2]], align 4
72-
// CHECK: call void @"??$template_fn@$$BY03M@@YAXY03M@Z"(ptr noundef byval([4 x float]) align 4 [[Tmp2]])
73-
// CHECK: store ptr {{.*}}, ptr [[Tmp3]], align 4
74-
// CHECK: call void @"??$template_fn@$$BY02H@@YAXY02H@Z"(ptr noundef byval([3 x i32]) align 4 [[Tmp3]])
70+
// CHECK: call void @llvm.memcpy.p0.p0.i32(ptr align 4 [[Tmp1]], ptr align 4 [[FA2]], i32 8, i1 false)
71+
// CHECK: call void @_Z11template_fnIA2_fEvT_(ptr noundef byval([2 x float]) align 4 [[Tmp1]])
72+
// CHECK: call void @llvm.memcpy.p0.p0.i32(ptr align 4 [[Tmp2]], ptr align 4 [[FA4]], i32 16, i1 false)
73+
// CHECK: call void @_Z11template_fnIA4_fEvT_(ptr noundef byval([4 x float]) align 4 [[Tmp2]])
74+
// CHECK: call void @llvm.memcpy.p0.p0.i32(ptr align 4 [[Tmp3]], ptr align 4 [[IA3]], i32 12, i1 false)
75+
// CHECK: call void @_Z11template_fnIA3_iEvT_(ptr noundef byval([3 x i32]) align 4 [[Tmp3]])
7576

7677
template<typename T>
7778
void template_fn(T Val) {}
@@ -82,6 +83,7 @@ void template_call(float FA2[2], float FA4[4], int IA3[3]) {
8283
template_fn(IA3);
8384
}
8485

86+
8587
// Verify that Array parameter element access correctly codegens.
8688
// CHECK-LABEL: define void {{.*}}element_access{{.*}}(ptr
8789
// CHECK-SAME: noundef byval([2 x float]) align 4 [[FA2:%[0-9A-Z]+]]

0 commit comments

Comments
 (0)