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

[Jessica Clarke via Phabricator via cfe-commits]

jrtc27 added a comment.

Seems good other than additional comments regarding code clarity.



================
Comment at: clang/lib/CodeGen/TargetInfo.cpp:10577
     NeededArgGPRs++;
-  return IsCandidate;
+  return true;
 }
----------------
This NFC hunk definitely makes this clearer :)


================
Comment at: clang/lib/CodeGen/TargetInfo.cpp:10603
       CharUnits::fromQuantity(getDataLayout().getABITypeAlignment(Field2Ty));
-  CharUnits Field1Size =
-      CharUnits::fromQuantity(getDataLayout().getTypeStoreSize(Field1Ty));
-  CharUnits Field2OffNoPadNoPack = Field1Size.alignTo(Field2Align);
+  CharUnits Field1EffectiveSize =
+      CharUnits::fromQuantity(getDataLayout().getTypeStoreSize(Field1Ty)) +
----------------
lenary wrote:
> 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. :)
+1 to Field1End or similar


================
Comment at: clang/lib/CodeGen/TargetInfo.cpp:10604-10605
+  CharUnits Field1EffectiveSize =
+      CharUnits::fromQuantity(getDataLayout().getTypeStoreSize(Field1Ty)) +
+      Field1Off;
+  CharUnits Field2OffNoPadNoPack = Field1EffectiveSize.alignTo(Field2Align);
----------------
Swapping the order of the operands would be the more natural way to express this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91270