[Clang][CodeGen][RISCV] Fix hard float ABI test cases with empty struct

[luismarques]

Summary

The code seemed not to account for the field 1 offset. This patch hopefully fixes that issue.

[lenary]

I *think* this is looking good, but I'd like a second pair of eyes from @jrtc27 as I know she's fixed bugs around this area recently too. We would like to have this ready for 11.0.1 if possible.

clang/lib/CodeGen/TargetInfo.cpp

10595

Please may you update this name to reflect what it really is - the offset of the *end* of Field1, rather than its size. This code is complex enough without confusing names. :)

[jrtc27]

Seems good other than additional comments regarding code clarity.

clang/lib/CodeGen/TargetInfo.cpp

10567

This NFC hunk definitely makes this clearer :)

10595

+1 to Field1End or similar

10596–10597

Swapping the order of the operands would be the more natural way to express this.

[luismarques]

Address review feedback.

[lenary]

LGTM! Please land these tonight so we can have them backported.

clang/lib/CodeGen/TargetInfo.cpp

 // NeededArgGPRs are incremented appropriately.
 bool RISCVABIInfo::detectFPCCEligibleStruct(QualType Ty, llvm::Type *&Field1Ty,
                                             CharUnits &Field1Off,
                                             llvm::Type *&Field2Ty,
                                             CharUnits &Field2Off,
                                             int &NeededArgGPRs,
                                             int &NeededArgFPRs) const {
   Field1Ty = nullptr;
   Field2Ty = nullptr;
   NeededArgGPRs = 0;
   NeededArgFPRs = 0;
   bool IsCandidate = detectFPCCEligibleStructHelper(
       Ty, CharUnits::Zero(), Field1Ty, Field1Off, Field2Ty, Field2Off);
   // Not really a candidate if we have a single int but no float.
   if (Field1Ty && !Field2Ty && !Field1Ty->isFloatingPointTy())
     return false;
   if (!IsCandidate)
     return false;
   if (Field1Ty && Field1Ty->isFloatingPointTy())
     NeededArgFPRs++;
   else if (Field1Ty)
     NeededArgGPRs++;
   if (Field2Ty && Field2Ty->isFloatingPointTy())
     NeededArgFPRs++;
   else if (Field2Ty)
     NeededArgGPRs++;
-  return IsCandidate;
+  return true;
 }
 
 // Call getCoerceAndExpand for the two-element flattened struct described by
 // Field1Ty, Field1Off, Field2Ty, Field2Off. This method will create an
 // appropriate coerceToType and unpaddedCoerceToType.
 ABIArgInfo RISCVABIInfo::coerceAndExpandFPCCEligibleStruct(
     llvm::Type *Field1Ty, CharUnits Field1Off, llvm::Type *Field2Ty,
     CharUnits Field2Off) const {
ABIArgInfo RISCVABIInfo::coerceAndExpandFPCCEligibleStruct(
   if (!Field2Ty) {
     return ABIArgInfo::getCoerceAndExpand(
         llvm::StructType::get(getVMContext(), CoerceElts, !Field1Off.isZero()),
         UnpaddedCoerceElts[0]);
   }
 
   CharUnits Field2Align =
       CharUnits::fromQuantity(getDataLayout().getABITypeAlignment(Field2Ty));
-  CharUnits Field1Size =
+  CharUnits Field1End = Field1Off +
       CharUnits::fromQuantity(getDataLayout().getTypeStoreSize(Field1Ty));
-  CharUnits Field2OffNoPadNoPack = Field1Size.alignTo(Field2Align);
+  CharUnits Field2OffNoPadNoPack = Field1End.alignTo(Field2Align);
 
   CharUnits Padding = CharUnits::Zero();
   if (Field2Off > Field2OffNoPadNoPack)
     Padding = Field2Off - Field2OffNoPadNoPack;
-  else if (Field2Off != Field2Align && Field2Off > Field1Size)
-    Padding = Field2Off - Field1Size;
+  else if (Field2Off != Field2Align && Field2Off > Field1End)
+    Padding = Field2Off - Field1End;
 
   bool IsPacked = !Field2Off.isMultipleOf(Field2Align);
 
   if (!Padding.isZero())
     CoerceElts.push_back(llvm::ArrayType::get(
         llvm::Type::getInt8Ty(getVMContext()), Padding.getQuantity()));
 
   CoerceElts.push_back(Field2Ty);

clang/test/CodeGen/riscv32-ilp32d-abi.cpp

 // RUN: %clang_cc1 -triple riscv32 -target-feature +d -target-abi ilp32d \
 // RUN:     -Wno-missing-declarations -emit-llvm %s -o - | FileCheck %s
 
 struct empty_float2 { struct {}; float f; float g; };
 
 // CHECK: define float @_Z14f_empty_float212empty_float2(float %0, float %1)
-// FIXME: Extraneous padding before the second float
-// CHECK: { [4 x i8], float, [4 x i8], float }
+// CHECK: { [4 x i8], float, float }
 float f_empty_float2(empty_float2 a) {
     return a.g;
 }
 
 struct empty_double2 { struct {}; double f; double g; };
 
 // CHECK: define double @_Z15f_empty_double213empty_double2(double %0, double %1)
-// FIXME: Extraneous padding before the second double
-// CHECK: { [8 x i8], double, [8 x i8], double }
+// CHECK: { [8 x i8], double, double }
 double f_empty_double2(empty_double2 a) {
     return a.g;
 }
 
 struct empty_float_double { struct {}; float f; double g; };
 
 // CHECK: define double @_Z20f_empty_float_double18empty_float_double(float %0, double %1)
 // CHECK: { [4 x i8], float, double }
 double f_empty_float_double(empty_float_double a) {
     return a.g;
 }
 
 struct empty_double_float { struct {}; double f; float g; };
 
 // CHECK: define double @_Z20f_empty_double_float18empty_double_float(double %0, float %1)
-// FIXME: Extraneous padding before the float
-// CHECK: { [8 x i8], double, [8 x i8], float }
+// CHECK: { [8 x i8], double, float }
 double f_empty_double_float(empty_double_float a) {
     return a.g;
 }