self review continued. Make FlattenAccessAndTypes not recursive and handle records correctly.

Author: spall

clang/lib/CodeGen/CGExpr.cpp

@@ -6361,62 +6361,87 @@ LValue CodeGenFunction::EmitPseudoObjectLValue(const PseudoObjectExpr *E) {
 }
 
 void CodeGenFunction::FlattenAccessAndType(
-    Address Val, QualType SrcTy, SmallVector<llvm::Value *, 4> &IdxList,
-    SmallVector<std::pair<Address, llvm::Value *>, 16> &GEPList,
-    SmallVector<QualType> &FlatTypes) {
+    Address Addr, QualType AddrType,
+    SmallVectorImpl<std::pair<Address, llvm::Value *>> &AccessList,
+    SmallVectorImpl<QualType> &FlatTypes) {
+  // WorkList is list of type we are processing + the Index List to access
+  // the field of that type in Addr for use in a GEP
+  llvm::SmallVector<std::pair<QualType, llvm::SmallVector<llvm::Value *, 4>>,
+                    16>
+      WorkList;
   llvm::IntegerType *IdxTy = llvm::IntegerType::get(getLLVMContext(), 32);
-  if (const ConstantArrayType *CAT = dyn_cast<ConstantArrayType>(SrcTy)) {
-    uint64_t Size = CAT->getZExtSize();
-    for (unsigned i = 0; i < Size; i++) {
-      // flatten each member of the array
-      // add index of this element to index list
-      llvm::Value *Idx = llvm::ConstantInt::get(IdxTy, i);
-      IdxList.push_back(Idx);
-      // recur on this object
-      FlattenAccessAndType(Val, CAT->getElementType(), IdxList, GEPList,
-                           FlatTypes);
-      // remove index of this element from index list
-      IdxList.pop_back();
-    }
-  } else if (const RecordType *RT = SrcTy->getAs<RecordType>()) {
-    RecordDecl *Record = RT->getDecl();
-    const CGRecordLayout &RL = getTypes().getCGRecordLayout(Record);
-    // do I need to check if its a cxx record decl?
-
-    for (auto fieldIter = Record->field_begin(), fieldEnd = Record->field_end();
-         fieldIter != fieldEnd; ++fieldIter) {
-      // get the field number
-      unsigned FieldNum = RL.getLLVMFieldNo(*fieldIter);
-      // can we just do *fieldIter->getFieldIndex();
-      // add that index to the index list
-      llvm::Value *Idx = llvm::ConstantInt::get(IdxTy, FieldNum);
-      IdxList.push_back(Idx);
-      // recur on the field
-      FlattenAccessAndType(Val, fieldIter->getType(), IdxList, GEPList,
-                           FlatTypes);
-      // remove index of this element from index list
-      IdxList.pop_back();
-    }
-  } else if (const VectorType *VT = SrcTy->getAs<VectorType>()) {
-    llvm::Type *VTy = ConvertTypeForMem(SrcTy);
-    CharUnits Align = getContext().getTypeAlignInChars(SrcTy);
-    Address GEP =
-        Builder.CreateInBoundsGEP(Val, IdxList, VTy, Align, "vector.gep");
-    for (unsigned i = 0; i < VT->getNumElements(); i++) {
-      // add index to the list
-      llvm::Value *Idx = llvm::ConstantInt::get(IdxTy, i);
-      // create gep. no need to recur since its always a scalar
-      // gep on vector is not recommended so combine gep with extract/insert
-      GEPList.push_back({GEP, Idx});
-      FlatTypes.push_back(VT->getElementType());
+  WorkList.push_back(
+      {AddrType,
+       {llvm::ConstantInt::get(
+           IdxTy,
+           0)}}); // Addr should be a pointer so we need to 'dereference' it
+
+  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;
+    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});
+      }
+    } 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;
+      }
+      const CXXRecordDecl *CXXD = dyn_cast<CXXRecordDecl>(Record);
+
+      llvm::SmallVector<QualType, 16> FieldTypes;
+      if (CXXD && CXXD->isStandardLayout())
+        Record = CXXD->getStandardLayoutBaseWithFields();
+
+      // deal with potential base classes
+      if (CXXD && !CXXD->isStandardLayout()) {
+        for (auto &Base : CXXD->bases())
+          FieldTypes.push_back(Base.getType());
+      }
+
+      for (auto *FD : Record->fields())
+        FieldTypes.push_back(FD->getType());
+
+      for (int64_t i = FieldTypes.size() - 1; i > -1; i--) {
+        llvm::SmallVector<llvm::Value *, 4> IdxListCopy = IdxList;
+        IdxListCopy.push_back(llvm::ConstantInt::get(IdxTy, i));
+        WorkList.insert(WorkList.end(), {FieldTypes[i], IdxListCopy});
+      }
+    } else if (const auto *VT = dyn_cast<VectorType>(T)) {
+      llvm::Type *LLVMT = ConvertTypeForMem(T);
+      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);
+        // gep on vector fields is not recommended so combine gep with
+        // extract/insert
+        AccessList.push_back({GEP, Idx});
+        FlatTypes.push_back(VT->getElementType());
+      }
+    } else {
+      // a scalar/builtin type
+      llvm::Type *LLVMT = ConvertTypeForMem(T);
+      CharUnits Align = getContext().getTypeAlignInChars(T);
+      Address GEP =
+          Builder.CreateInBoundsGEP(Addr, IdxList, LLVMT, Align, "gep");
+      AccessList.push_back({GEP, NULL});
+      FlatTypes.push_back(T);
     }
-  } else { // should be a scalar should we assert or check?
-    // create a gep
-    llvm::Type *Ty = ConvertTypeForMem(SrcTy);
-    CharUnits Align = getContext().getTypeAlignInChars(SrcTy);
-    Address GEP = Builder.CreateInBoundsGEP(Val, IdxList, Ty, Align, "gep");
-    GEPList.push_back({GEP, NULL});
-    FlatTypes.push_back(SrcTy);
-  }
-  // target extension types?
+  }
 }

clang/lib/CodeGen/CGExprAgg.cpp

@@ -496,14 +496,10 @@ static void EmitHLSLScalarFlatCast(CodeGenFunction &CGF, Address DestVal,
                                    QualType DestTy, llvm::Value *SrcVal,
                                    QualType SrcTy, SourceLocation Loc) {
   // Flatten our destination
-  SmallVector<QualType> DestTypes; // Flattened type
-  SmallVector<llvm::Value *, 4> IdxList;
-  IdxList.push_back(
-      llvm::ConstantInt::get(llvm::IntegerType::get(CGF.getLLVMContext(), 32),
-                             0)); // because an Address is a pointer
+  SmallVector<QualType, 16> DestTypes; // Flattened type
   SmallVector<std::pair<Address, llvm::Value *>, 16> StoreGEPList;
   // ^^ Flattened accesses to DestVal we want to store into
-  CGF.FlattenAccessAndType(DestVal, DestTy, IdxList, StoreGEPList, DestTypes);
+  CGF.FlattenAccessAndType(DestVal, DestTy, StoreGEPList, DestTypes);
 
   if (const VectorType *VT = SrcTy->getAs<VectorType>()) {
     SrcTy = VT->getElementType();
@@ -536,23 +532,15 @@ static void EmitHLSLAggregateFlatCast(CodeGenFunction &CGF, Address DestVal,
                                       QualType DestTy, Address SrcVal,
                                       QualType SrcTy, SourceLocation Loc) {
   // Flatten our destination
-  SmallVector<QualType> DestTypes; // Flattened type
-  SmallVector<llvm::Value *, 4> IdxList;
-  IdxList.push_back(
-      llvm::ConstantInt::get(llvm::IntegerType::get(CGF.getLLVMContext(), 32),
-                             0)); // Because an Address is a pointer
+  SmallVector<QualType, 16> DestTypes; // Flattened type
   SmallVector<std::pair<Address, llvm::Value *>, 16> StoreGEPList;
   // ^^ Flattened accesses to DestVal we want to store into
-  CGF.FlattenAccessAndType(DestVal, DestTy, IdxList, StoreGEPList, DestTypes);
+  CGF.FlattenAccessAndType(DestVal, DestTy, StoreGEPList, DestTypes);
   // Flatten our src
-  SmallVector<QualType> SrcTypes; // Flattened type
+  SmallVector<QualType, 16> SrcTypes; // Flattened type
   SmallVector<std::pair<Address, llvm::Value *>, 16> LoadGEPList;
   // ^^ Flattened accesses to SrcVal we want to load from
-  IdxList.clear();
-  IdxList.push_back(
-      llvm::ConstantInt::get(llvm::IntegerType::get(CGF.getLLVMContext(), 32),
-                             0)); // Because an Address is a pointer
-  CGF.FlattenAccessAndType(SrcVal, SrcTy, IdxList, LoadGEPList, SrcTypes);
+  CGF.FlattenAccessAndType(SrcVal, SrcTy, LoadGEPList, SrcTypes);
 
   assert(StoreGEPList.size() <= LoadGEPList.size() &&
          "Cannot perform HLSL flat cast when flattened source object \

clang/lib/CodeGen/CGExprScalar.cpp

@@ -2266,13 +2266,9 @@ bool CodeGenFunction::ShouldNullCheckClassCastValue(const CastExpr *CE) {
 static Value *EmitHLSLAggregateFlatCast(CodeGenFunction &CGF, Address RHSVal,
                                         QualType RHSTy, QualType LHSTy,
                                         SourceLocation Loc) {
-  SmallVector<llvm::Value *, 4> IdxList;
-  IdxList.push_back(
-      llvm::ConstantInt::get(llvm::IntegerType::get(CGF.getLLVMContext(), 32),
-                             0)); // because an Address is a pointer
   SmallVector<std::pair<Address, llvm::Value *>, 16> LoadGEPList;
-  SmallVector<QualType> SrcTypes; // Flattened type
-  CGF.FlattenAccessAndType(RHSVal, RHSTy, IdxList, LoadGEPList, SrcTypes);
+  SmallVector<QualType, 16> SrcTypes; // Flattened type
+  CGF.FlattenAccessAndType(RHSVal, RHSTy, LoadGEPList, SrcTypes);
   // 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>()) {

clang/lib/CodeGen/CodeGenFunction.h

@@ -4360,9 +4360,9 @@ class CodeGenFunction : public CodeGenTypeCache {
   LValue EmitPseudoObjectLValue(const PseudoObjectExpr *e);
 
   void FlattenAccessAndType(
-      Address Val, QualType SrcTy, SmallVector<llvm::Value *, 4> &IdxList,
-      SmallVector<std::pair<Address, llvm::Value *>, 16> &GEPList,
-      SmallVector<QualType> &FlatTypes);
+      Address Addr, QualType AddrTy,
+      SmallVectorImpl<std::pair<Address, llvm::Value *>> &AccessList,
+      SmallVectorImpl<QualType> &FlatTypes);
 
   llvm::Value *EmitIvarOffset(const ObjCInterfaceDecl *Interface,
                               const ObjCIvarDecl *Ivar);