bcain added a comment.
In general, I recommend qualifying metric and threshold values with their
associated units: bits, bytes, pages, etc. That way it's easy to see where the
unit conversions are happening and easy to see whether the logic is correct
without reasoning about how the value changes among lines.
Constants specified by the ABI should be designated with names and units
[unless maybe this conflicts w/llvm project naming conventions?]
================
Comment at: clang/lib/CodeGen/TargetInfo.cpp:7592
+ uint64_t Size = getContext().getTypeSize(Ty);
+ if (Size <= 64)
+ HexagonAdjustRegsLeft(Size, RegsLeft);
----------------
This `64` should probably be named something like
`HEXAGON_REGISTER_SIZE_BITS_MAX`?
================
Comment at: clang/lib/CodeGen/TargetInfo.cpp:7613
+ if (HexagonAdjustRegsLeft(Size, RegsLeft))
+ Align = Size <= 32 ? 32 : 64;
+ if (Size <= Align) {
----------------
`32` here should be `HEXAGON_REGISTER_SIZE_BITS_MIN` maybe?
================
Comment at: clang/lib/CodeGen/TargetInfo.cpp:7705
+ // Round up to the minimum stack alignment for varargs which is 4 bytes.
+ uint64_t Offset = llvm::alignTo(CGF.getContext().getTypeSize(Ty) / 8, 4);
+
----------------
I suggest:
```
uint64_t Offset = llvm::alignTo(CGF.getContext().getTypeSize(Ty) / CHAR_BIT,
HEXAGON_MIN_STACK_ALIGN_BYTES);
```
================
Comment at: clang/lib/CodeGen/TargetInfo.cpp:7748
+ QualType Ty) const {
+ int ArgSize = CGF.getContext().getTypeSize(Ty) / 8;
+
----------------
```
int ArgSizeBytes = CGF.getContext().getTypeSize(Ty) / CHAR_BIT;
if (ArgSizeBytes > HEXAGON_VA_REGISTER_LIMIT_BYTES)
return EmitVAArgFromMemory(CGF, VAListAddr, Ty);
```
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D75638/new/
https://reviews.llvm.org/D75638
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
