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

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.


================
Comment at: lib/Basic/Targets/RISCV.h:102
     // TODO: support lp64f and lp64d ABIs.
-    if (Name == "lp64") {
+    if (Name == "lp64" || Name == "lp64f" || Name == "lp64d") {
       ABI = Name;
----------------
You can remove the TODO here, assuming that this CC support is all that's necessary to support these ABIs.


================
Comment at: lib/CodeGen/TargetInfo.cpp:9223
+
+  bool IsInt = Ty->isIntegralOrEnumerationType();
+  bool IsFloat = Ty->isRealFloatingType();
----------------
Should this include pointers?  Pointers are often interchangeably with integers by ABIs.

The same question also applies to C++ references and data-member-pointer types, and maybe others that I'm not thinking of.


================
Comment at: lib/CodeGen/TargetInfo.cpp:9298
+    return true;
+  }
+
----------------
This definitely needs to handle bit-fields in some way.


================
Comment at: lib/CodeGen/TargetInfo.cpp:9352
+      CharUnits::fromQuantity(getDataLayout().getTypeStoreSize(Field1Ty));
+  CharUnits Field2OffNoPadNoPack = std::max(Field2Align, Field1Size);
+
----------------
This should use `alignTo`, not `max`.  It's possible that they're equivalent for the narrow cases that can come out of all the checks above, but it's both clearer and safer to use the correct computation.


================
Comment at: lib/CodeGen/TargetInfo.cpp:9374
+
+  return ABIArgInfo::getCoerceAndExpand(CoerceToType, UnpaddedCoerceToType);
+}
----------------
This seems like a reasonable use of coerce-and-expand.


Repository:
  rC Clang

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

https://reviews.llvm.org/D60456