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
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
