addressing pr comments

Author: spall

clang/include/clang/AST/ASTContext.h

@@ -1471,10 +1471,6 @@ class ASTContext : public RefCountedBase<ASTContext> {
   /// type to the decayed type.
   QualType getDecayedType(QualType Orig, QualType Decayed) const;
 
-  /// Return the uniqued reference to a constant array type from the
-  /// original array parameter type.
-  QualType getConstantArrayFromArrayParameterType(QualType Ty) const;
-
   /// Return the uniqued reference to a specified array parameter type from the
   /// original array type.
   QualType getArrayParameterType(QualType Ty) const;

clang/include/clang/AST/Type.h

@@ -3757,6 +3757,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

clang/lib/AST/ASTContext.cpp

@@ -3839,16 +3839,6 @@ QualType ASTContext::getDecayedType(QualType T) const {
   return getDecayedType(T, Decayed);
 }
 
-QualType ASTContext::getConstantArrayFromArrayParameterType(QualType Ty) const {
-  if (Ty->isConstantArrayType() && !Ty->isArrayParameterType())
-    return Ty;
-  assert(Ty->isArrayParameterType() && "Ty must be an array parameter type.");
-  const auto *ATy = cast<ArrayParameterType>(Ty);
-  return getConstantArrayType(ATy->getElementType(), ATy->getSize(),
-                              ATy->getSizeExpr(), ATy->getSizeModifier(),
-                              ATy->getIndexTypeQualifiers().getAsOpaqueValue());
-}
-
 QualType ASTContext::getArrayParameterType(QualType Ty) const {
   if (Ty->isArrayParameterType())
     return Ty;

clang/lib/AST/Type.cpp

@@ -240,6 +240,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,

clang/lib/CodeGen/CGCall.cpp

@@ -4726,16 +4726,17 @@ void CodeGenFunction::EmitCallArg(CallArgList &args, const Expr *E,
     return emitWritebackArg(*this, args, CRE);
   }
 
-  assert(type->isArrayParameterType() ||
-         (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);

clang/lib/CodeGen/CGExpr.cpp

@@ -5823,9 +5823,9 @@ LValue CodeGenFunction::EmitBinaryOperatorLValue(const BinaryOperator *E) {
 LValue CodeGenFunction::EmitHLSLArrayAssignLValue(const BinaryOperator *E) {
   // Don't emit an LValue for the RHS because it might not be an LValue
   LValue LHS = EmitLValue(E->getLHS());
-  // In C assignment operator RHS is often an RValue.
-  // EmitAggregateAssign expects an LValue for the RHS so call the below
-  // function instead.
+  // 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;
 }

clang/lib/Sema/Sema.cpp

@@ -726,6 +726,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;
 

clang/lib/Sema/SemaExprCXX.cpp

@@ -4437,7 +4437,11 @@ Sema::PerformImplicitConversion(Expr *From, QualType ToType,
                                /*BasePath=*/nullptr, CCK)
                  .get();
     } else { // FromType must be ArrayParameterType
-      FromType = Context.getConstantArrayFromArrayParameterType(FromType);
+      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();

clang/lib/Sema/SemaOverload.cpp

@@ -2244,6 +2244,10 @@ static bool IsStandardConversion(Sema &S, Expr* From, QualType ToType,
   }
 
   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
@@ -2253,7 +2257,8 @@ static bool IsStandardConversion(Sema &S, Expr* From, QualType ToType,
       FromType = S.Context.getArrayParameterType(FromType);
       SCS.First = ICK_HLSL_Array_RValue;
     } else if (FromType->isArrayParameterType()) {
-      FromType = S.Context.getConstantArrayFromArrayParameterType(FromType);
+      const ArrayParameterType *APT = cast<ArrayParameterType>(FromType);
+      FromType = APT->getConstantArrayType(S.Context);
       SCS.First = ICK_HLSL_Array_RValue;
     } else {
       SCS.First = ICK_Identity;

clang/test/CodeGenHLSL/ArrayTemporary.hlsl

@@ -47,7 +47,7 @@ void call3() {
 
 // CHECK-LABEL: define void {{.*}}call4{{.*}}(ptr
 // CHECK: [[Tmp:%.*]] = alloca [2 x [2 x float]]
-// CHECK: store ptr {{.*}}, ptr [[Tmp]], align 4
+// CHECK: call void @llvm.memcpy.p0.p0.i32(ptr align 4 [[Tmp]], ptr align 4 [[Arr]], i32 16, i1 false)
 // CHECK: call void {{.*}}fn3{{.*}}(ptr noundef byval([2 x [2 x float]]) align 4 [[Tmp]])
 
 void call4(float Arr[2][2]) {
@@ -66,12 +66,12 @@ void call4(float Arr[2][2]) {
 // CHECK: [[Tmp1:%.*]] = alloca [2 x float]
 // CHECK: [[Tmp2:%.*]] = alloca [4 x float]
 // CHECK: [[Tmp3:%.*]] = alloca [3 x i32]
-// CHECK: store ptr {{.*}}, ptr [[Tmp1]], align 4
-// CHECK: call void @"??$template_fn@$$BY01M@@YAXY01M@Z"(ptr noundef byval([2 x float]) align 4 [[Tmp1]])
-// CHECK: store ptr {{.*}}, ptr [[Tmp2]], align 4
-// CHECK: call void @"??$template_fn@$$BY03M@@YAXY03M@Z"(ptr noundef byval([4 x float]) align 4 [[Tmp2]])
-// CHECK: store ptr {{.*}}, ptr [[Tmp3]], align 4
-// CHECK: call void @"??$template_fn@$$BY02H@@YAXY02H@Z"(ptr noundef byval([3 x i32]) align 4 [[Tmp3]])
+// CHECK: call void @llvm.memcpy.p0.p0.i32(ptr align 4 [[Tmp1]], ptr align 4 [[FA2]], i32 8, i1 false)
+// CHECK: call void @_Z11template_fnIA2_fEvT_(ptr noundef byval([2 x float]) align 4 [[Tmp1]])
+// CHECK: call void @llvm.memcpy.p0.p0.i32(ptr align 4 [[Tmp2]], ptr align 4 [[FA4]], i32 16, i1 false)
+// CHECK: call void @_Z11template_fnIA4_fEvT_(ptr noundef byval([4 x float]) align 4 [[Tmp2]])
+// CHECK: call void @llvm.memcpy.p0.p0.i32(ptr align 4 [[Tmp3]], ptr align 4 [[IA3]], i32 12, i1 false)
+// CHECK: call void @_Z11template_fnIA3_iEvT_(ptr noundef byval([3 x i32]) align 4 [[Tmp3]])
 
 template<typename T>
 void template_fn(T Val) {}