jasonliu added inline comments.
================ Comment at: clang/include/clang/AST/ASTContext.h:259 mutable TypeInfoMap MemoizedTypeInfo; + /// /// A cache from types to size and preferred alignment information. + mutable TypeInfoMap MemoizedPreferredTypeInfo; ---------------- nit: Extra /// at the beginning of the comment could be removed. ================ Comment at: clang/include/clang/AST/ASTContext.h:2133 + /// 'arm_sve_vector_bits'. Should only be called if T->isVLST(). + unsigned getBitwidthForAttributedSveType(const Type *T) const; + ---------------- Maybe you want to move up to a newer base? I don't see how this change belong to this patch. ================ Comment at: clang/lib/AST/ASTContext.cpp:1872 - // This call can invalidate MemoizedTypeInfo[T], so we need a second lookup. - TypeInfo TI = getTypeInfoImpl(T); - MemoizedTypeInfo[T] = TI; - return TI; + // This call can invalidate MemoizedTypeInfo[T], so we need a second lookup. + TypeInfo TI = getTypeInfoImpl(T, NeedsPreferredAlignment); ---------------- nit: This comment needs an update, as it could invalidate MemoizedPreferredTypeInfo depending on the flag. ================ Comment at: clang/lib/AST/ASTContext.cpp:2106 Width = Target->getDoubleWidth(); - Align = Target->getDoubleAlign(); + setAlign(Width, Target->getDoubleAlign()); break; ---------------- I have two concerns here: 1. I feel like we are having duplicate logic between here and ASTContext::getPreferredTypeAlign for how to get preferred alignment. 2. These logic are too AIX-centric and only modified the places for where types are affected by preferred alignment on AIX. What if on some platforms, BuiltinType::Float have different preferred alignments? Or Width is not the preferred alignment on certain platform for double/long double. ================ Comment at: clang/lib/CodeGen/TargetInfo.cpp:4539 CharUnits CCAlign = getParamTypeAlignment(Ty); CharUnits TyAlign = getContext().getTypeAlignInChars(Ty); ---------------- Question: It looks like getNaturalAlignIndirect and getTypeAlignInChars here are all returning ABI alignment. But according to the comments, we should use a preferred alignment when it's a complete object. Isn't this complete object? Or I'm missing something? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D86790/new/ https://reviews.llvm.org/D86790 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits