[PATCH] D48044: [Power9] Update fp128 as a valid homogenous aggregate base type
Nemanja Ivanovic via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Wed Jul 4 07:31:21 PDT 2018
nemanjai accepted this revision.
nemanjai added a comment.
This revision is now accepted and ready to land.
Other than a few style nits that can be fixed on the commit, this LGTM.
================
Comment at: include/clang/AST/Type.h:1802
bool isFloat16Type() const; // C11 extension ISO/IEC TS 18661
+ bool isFloat128Type() const;
bool isRealType() const; // C99 6.2.5p17 (real floating + integer)
----------------
// IEEE 754 binary128
================
Comment at: lib/CodeGen/TargetInfo.cpp:4609
// Homogeneous aggregates for ELFv2 must have base types of float,
// double, long double, or 128-bit vectors.
if (const BuiltinType *BT = Ty->getAs<BuiltinType>()) {
----------------
This comment should probably be updated.
================
Comment at: lib/CodeGen/TargetInfo.cpp:4633
uint32_t NumRegs =
- Base->isVectorType() ? 1 : (getContext().getTypeSize(Base) + 63) / 64;
+ ((getContext().getTargetInfo().hasFloat128Type() &&
+ Base->isFloat128Type()) ||
----------------
This expression looks very messy, I think it's probably better to rewrite it as multiple expressions or an `if` statement.
================
Comment at: test/CodeGen/ppc64le-f128Aggregates.c:19
+
+struct fp2a2b { __float128 a[2]; __float128 b[2]; };
+
----------------
Is it still a homogeneous aggregate if it's nested?
i.e. `struct fp10 { struct fp2 a, struct fp3 b };`
And if so, should we add that to the test?
https://reviews.llvm.org/D48044
More information about the cfe-commits
mailing list