respond to pr comments

Author: spall

clang/lib/CodeGen/CGExpr.cpp

@@ -6385,23 +6385,15 @@ void CodeGenFunction::FlattenAccessAndType(
     assert(!isa<MatrixType>(T) && "Matrix types not yet supported in HLSL");
     if (const auto *CAT = dyn_cast<ConstantArrayType>(T)) {
       uint64_t Size = CAT->getZExtSize();
-      for (int64_t i = Size - 1; i > -1; i--) {
+      for (int64_t I = Size - 1; I > -1; I--) {
         llvm::SmallVector<llvm::Value *, 4> IdxListCopy = IdxList;
-        IdxListCopy.push_back(llvm::ConstantInt::get(IdxTy, i));
+        IdxListCopy.push_back(llvm::ConstantInt::get(IdxTy, I));
         WorkList.insert(WorkList.end(), {CAT->getElementType(), IdxListCopy});
       }
     } else if (const auto *RT = dyn_cast<RecordType>(T)) {
       const RecordDecl *Record = RT->getDecl();
-      if (Record->isUnion()) {
-        IdxList.push_back(llvm::ConstantInt::get(IdxTy, 0));
-        llvm::Type *LLVMT = ConvertTypeForMem(T);
-        CharUnits Align = getContext().getTypeAlignInChars(T);
-        Address GEP =
-            Builder.CreateInBoundsGEP(Addr, IdxList, LLVMT, Align, "union.gep");
-        AccessList.push_back({GEP, NULL});
-        FlatTypes.push_back(T);
-        continue;
-      }
+      assert(!Record->isUnion() && "Union types not supported in flat cast.");
+
       const CXXRecordDecl *CXXD = dyn_cast<CXXRecordDecl>(Record);
 
       llvm::SmallVector<QualType, 16> FieldTypes;

clang/lib/CodeGen/CGExprAgg.cpp

@@ -501,30 +501,28 @@ static void EmitHLSLScalarFlatCast(CodeGenFunction &CGF, Address DestVal,
   // ^^ Flattened accesses to DestVal we want to store into
   CGF.FlattenAccessAndType(DestVal, DestTy, StoreGEPList, DestTypes);
 
-  if (const VectorType *VT = SrcTy->getAs<VectorType>()) {
-    SrcTy = VT->getElementType();
-    assert(StoreGEPList.size() <= VT->getNumElements() &&
-           "Cannot perform HLSL flat cast when vector source \
-           object has less elements than flattened destination \
-           object.");
-    for (unsigned i = 0; i < StoreGEPList.size(); i++) {
-      llvm::Value *Load =
-          CGF.Builder.CreateExtractElement(SrcVal, i, "vec.load");
-      llvm::Value *Cast =
-          CGF.EmitScalarConversion(Load, SrcTy, DestTypes[i], Loc);
-
-      // store back
-      llvm::Value *Idx = StoreGEPList[i].second;
-      if (Idx) {
-        llvm::Value *V =
-            CGF.Builder.CreateLoad(StoreGEPList[i].first, "load.for.insert");
-        Cast = CGF.Builder.CreateInsertElement(V, Cast, Idx);
-      }
-      CGF.Builder.CreateStore(Cast, StoreGEPList[i].first);
+  assert(SrcTy->isVectorType() && "HLSL Flat cast doesn't handle splatting.");
+  const VectorType *VT = SrcTy->getAs<VectorType>();
+  SrcTy = VT->getElementType();
+  assert(StoreGEPList.size() <= VT->getNumElements() &&
+         "Cannot perform HLSL flat cast when vector source \
+         object has less elements than flattened destination \
+         object.");
+  for (unsigned i = 0; i < StoreGEPList.size(); i++) {
+    llvm::Value *Load = CGF.Builder.CreateExtractElement(SrcVal, i, "vec.load");
+    llvm::Value *Cast =
+        CGF.EmitScalarConversion(Load, SrcTy, DestTypes[i], Loc);
+
+    // store back
+    llvm::Value *Idx = StoreGEPList[i].second;
+    if (Idx) {
+      llvm::Value *V =
+          CGF.Builder.CreateLoad(StoreGEPList[i].first, "load.for.insert");
+      Cast = CGF.Builder.CreateInsertElement(V, Cast, Idx);
     }
-    return;
+    CGF.Builder.CreateStore(Cast, StoreGEPList[i].first);
   }
-  llvm_unreachable("HLSL Flat cast doesn't handle splatting.");
+  return;
 }
 
 // emit a flat cast where the RHS is an aggregate

clang/lib/CodeGen/CGExprScalar.cpp

@@ -2792,11 +2792,10 @@ Value *ScalarExprEmitter::VisitCastExpr(CastExpr *CE) {
     SourceLocation Loc = CE->getExprLoc();
     QualType SrcTy = E->getType();
 
-    if (RV.isAggregate()) { // RHS is an aggregate
-      Address SrcVal = RV.getAggregateAddress();
-      return EmitHLSLAggregateFlatCast(CGF, SrcVal, SrcTy, DestTy, Loc);
-    }
-    llvm_unreachable("Not a valid HLSL Flat Cast.");
+    assert(RV.isAggregate() && "Not a valid HLSL Flat Cast.");
+    // RHS is an aggregate
+    Address SrcVal = RV.getAggregateAddress();
+    return EmitHLSLAggregateFlatCast(CGF, SrcVal, SrcTy, DestTy, Loc);
   }
   } // end of switch
 

clang/lib/Sema/SemaHLSL.cpp

@@ -2477,7 +2477,7 @@ bool SemaHLSL::CanPerformScalarCast(QualType SrcTy, QualType DestTy) {
 }
 
 // Can we perform an HLSL Flattened cast?
-// TODO: update this code when matrices are added
+// TODO: update this code when matrices are added; see issue #88060
 bool SemaHLSL::CanPerformAggregateCast(Expr *Src, QualType DestTy) {
 
   // Don't handle casts where LHS and RHS are any combination of scalar/vector
@@ -2500,11 +2500,20 @@ bool SemaHLSL::CanPerformAggregateCast(Expr *Src, QualType DestTy) {
   if (SrcTypes.size() < DestTypes.size())
     return false;
 
-  for (unsigned i = 0; i < DestTypes.size() && i < SrcTypes.size(); i++) {
-    if (!CanPerformScalarCast(SrcTypes[i], DestTypes[i])) {
+  unsigned I;
+  for (I = 0; I < DestTypes.size() && I < SrcTypes.size(); I++) {
+    if (SrcTypes[I]->isUnionType() || DestTypes[I]->isUnionType())
+      return false;
+    if (!CanPerformScalarCast(SrcTypes[I], DestTypes[I])) {
       return false;
     }
   }
+
+  // check the rest of the source type for unions.
+  for (; I < SrcTypes.size(); I++) {
+    if (SrcTypes[I]->isUnionType())
+      return false;
+  }
   return true;
 }