[PATCH] D60456: [RISCV][WIP/RFC] Hard float ABI support

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.


================
Comment at: clang/lib/CodeGen/TargetInfo.cpp:9232
+    if (IsFloat && Size > FLen)
+      return false;
+    // Can't be eligible if an integer type was already found (only fp+int or
----------------
Is this the only consideration for floating-point types?  Clang does have increasing support for `half` / various `float16` types.


================
Comment at: clang/lib/CodeGen/TargetInfo.cpp:9236
+    if (IsInt && Field1Ty && Field1Ty->isIntegerTy())
+      return false;
+    if (!Field1Ty) {
----------------
The comment here is wrong because fp+fp is allowed, right?

Is this not already caught by the post-processing checks you do in `detectFPCCEligibleStruct`?  Would it make more sense to just do all those checks there?


================
Comment at: clang/lib/CodeGen/TargetInfo.cpp:9258
+    assert(CurOff.isZero() && "Unexpected offset for first field");
+    Field2Ty = CGT.ConvertType(EltTy);
+    Field2Off = getContext().getTypeSizeInChars(EltTy);
----------------
`Field2Ty = Field1Ty`, please.


================
Comment at: clang/lib/CodeGen/TargetInfo.cpp:9288
+      return false;
+    for (const FieldDecl *FD : RD->fields()) {
+      const ASTRecordLayout &Layout = getContext().getASTRecordLayout(RD);
----------------
I really expect there to be something in this block about whether the field is a bit-field.  What exactly does your ABI specify if there's a bit-field?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60456/new/

https://reviews.llvm.org/D60456