address pr comments

Author: spall

clang/lib/AST/ExprConstant.cpp

@@ -14857,7 +14857,6 @@ bool IntExprEvaluator::VisitCastExpr(const CastExpr *E) {
   case CK_FixedPointCast:
   case CK_IntegralToFixedPoint:
   case CK_MatrixCast:
-    // TODO does CK_HLSLAggregateCast belong here?
     llvm_unreachable("invalid cast kind for integral value");
 
   case CK_BitCast:
@@ -14876,6 +14875,7 @@ bool IntExprEvaluator::VisitCastExpr(const CastExpr *E) {
   case CK_NoOp:
   case CK_LValueToRValueBitCast:
   case CK_HLSLArrayRValue:
+  case CK_HLSLAggregateCast:
     return ExprEvaluatorBaseTy::VisitCastExpr(E);
 
   case CK_MemberPointerToBoolean:

clang/lib/CodeGen/CGExpr.cpp

@@ -6370,25 +6370,19 @@ void CodeGenFunction::FlattenAccessAndType(
                     16>
       WorkList;
   llvm::IntegerType *IdxTy = llvm::IntegerType::get(getLLVMContext(), 32);
-  WorkList.push_back(
-      {AddrType,
-       {llvm::ConstantInt::get(
-           IdxTy,
-           0)}}); // Addr should be a pointer so we need to 'dereference' it
+  // Addr should be a pointer so we need to 'dereference' it
+  WorkList.push_back({AddrType, {llvm::ConstantInt::get(IdxTy, 0)}});
 
   while (!WorkList.empty()) {
-    std::pair<QualType, llvm::SmallVector<llvm::Value *, 4>> P =
-        WorkList.pop_back_val();
-    QualType T = P.first;
-    llvm::SmallVector<llvm::Value *, 4> IdxList = P.second;
+    auto [T, IdxList] = WorkList.pop_back_val();
     T = T.getCanonicalType().getUnqualifiedType();
     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--) {
         llvm::SmallVector<llvm::Value *, 4> IdxListCopy = IdxList;
         IdxListCopy.push_back(llvm::ConstantInt::get(IdxTy, I));
-        WorkList.insert(WorkList.end(), {CAT->getElementType(), IdxListCopy});
+        WorkList.emplace_back(CAT->getElementType(), IdxListCopy);
       }
     } else if (const auto *RT = dyn_cast<RecordType>(T)) {
       const RecordDecl *Record = RT->getDecl();
@@ -6419,8 +6413,8 @@ void CodeGenFunction::FlattenAccessAndType(
       CharUnits Align = getContext().getTypeAlignInChars(T);
       Address GEP =
           Builder.CreateInBoundsGEP(Addr, IdxList, LLVMT, Align, "vector.gep");
-      for (unsigned i = 0; i < VT->getNumElements(); i++) {
-        llvm::Value *Idx = llvm::ConstantInt::get(IdxTy, i);
+      for (unsigned I = 0, E = VT->getNumElements(); I < E; I++) {
+        llvm::Value *Idx = llvm::ConstantInt::get(IdxTy, I);
         // gep on vector fields is not recommended so combine gep with
         // extract/insert
         AccessList.push_back({GEP, Idx});
@@ -6432,7 +6426,7 @@ void CodeGenFunction::FlattenAccessAndType(
       CharUnits Align = getContext().getTypeAlignInChars(T);
       Address GEP =
           Builder.CreateInBoundsGEP(Addr, IdxList, LLVMT, Align, "gep");
-      AccessList.push_back({GEP, NULL});
+      AccessList.emplace_back(GEP, nullptr);
       FlatTypes.push_back(T);
     }
   }

clang/lib/CodeGen/CGExprAgg.cpp

@@ -508,19 +508,19 @@ static void EmitHLSLScalarFlatCast(CodeGenFunction &CGF, Address DestVal,
          "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");
+  for (unsigned I = 0, Size = StoreGEPList.size(); I < Size; I++) {
+    llvm::Value *Load = CGF.Builder.CreateExtractElement(SrcVal, I, "vec.load");
     llvm::Value *Cast =
-        CGF.EmitScalarConversion(Load, SrcTy, DestTypes[i], Loc);
+        CGF.EmitScalarConversion(Load, SrcTy, DestTypes[I], Loc);
 
     // store back
-    llvm::Value *Idx = StoreGEPList[i].second;
+    llvm::Value *Idx = StoreGEPList[I].second;
     if (Idx) {
       llvm::Value *V =
-          CGF.Builder.CreateLoad(StoreGEPList[i].first, "load.for.insert");
+          CGF.Builder.CreateLoad(StoreGEPList[I].first, "load.for.insert");
       Cast = CGF.Builder.CreateInsertElement(V, Cast, Idx);
     }
-    CGF.Builder.CreateStore(Cast, StoreGEPList[i].first);
+    CGF.Builder.CreateStore(Cast, StoreGEPList[I].first);
   }
   return;
 }
@@ -974,7 +974,7 @@ void AggExprEmitter::VisitCastExpr(CastExpr *E) {
     if (RV.isScalar()) {
       llvm::Value *SrcVal = RV.getScalarVal();
       EmitHLSLScalarFlatCast(CGF, DestVal, DestTy, SrcVal, SrcTy, Loc);
-    } else { // RHS is an aggregate
+    } else {
       assert(RV.isAggregate() &&
              "Can't perform HLSL Aggregate cast on a complex type.");
       Address SrcVal = RV.getAggregateAddress();

clang/lib/CodeGen/CGExprScalar.cpp

@@ -2272,24 +2272,25 @@ static Value *EmitHLSLAggregateFlatCast(CodeGenFunction &CGF, Address RHSVal,
   // LHS is either a vector or a builtin?
   // if its a vector create a temp alloca to store into and return that
   if (auto *VecTy = LHSTy->getAs<VectorType>()) {
+    assert(SrcTypes.size() >= VecTy->getNumElements() &&
+           "Flattened type on RHS must have more elements than vector on LHS.");
     llvm::Value *V =
         CGF.Builder.CreateLoad(CGF.CreateIRTemp(LHSTy, "flatcast.tmp"));
     // write to V.
-    for (unsigned i = 0; i < VecTy->getNumElements(); i++) {
-      llvm::Value *Load = CGF.Builder.CreateLoad(LoadGEPList[i].first, "load");
-      llvm::Value *Idx = LoadGEPList[i].second;
+    for (unsigned I = 0, E = VecTy->getNumElements(); I < E; I++) {
+      llvm::Value *Load = CGF.Builder.CreateLoad(LoadGEPList[I].first, "load");
+      llvm::Value *Idx = LoadGEPList[I].second;
       Load = Idx ? CGF.Builder.CreateExtractElement(Load, Idx, "vec.extract")
                  : Load;
       llvm::Value *Cast = CGF.EmitScalarConversion(
-          Load, SrcTypes[i], VecTy->getElementType(), Loc);
-      V = CGF.Builder.CreateInsertElement(V, Cast, i);
+          Load, SrcTypes[I], VecTy->getElementType(), Loc);
+      V = CGF.Builder.CreateInsertElement(V, Cast, I);
     }
     return V;
   }
   // i its a builtin just do an extract element or load.
   assert(LHSTy->isBuiltinType() &&
          "Destination type must be a vector or builtin type.");
-  // TODO add asserts about things being long enough
   llvm::Value *Load = CGF.Builder.CreateLoad(LoadGEPList[0].first, "load");
   llvm::Value *Idx = LoadGEPList[0].second;
   Load =