[PATCH] D48044: [Power9] Update fp128 as a valid homogenous aggregate base type

[Nemanja Ivanovic via Phabricator via cfe-commits]

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