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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
