[PATCH] D132926: [ARM64EC 12/?] Add param/ret attr for struct to help generate correct thunk for arm64ec

[Eli Friedman via Phabricator via llvm-commits]

efriedma added a comment.

Using "Sign" as an abbreviation for "Signature" is confusing.  Avoid abbreviating if you can; if you think it's necessary, use "Sig" as shorthand for "Signature".

Not sure it's the right thing to put in the name of the attribute, though; the name of the attribute should probably mention it's the "size" of the argument.  Maybe just arm64ec_arg_size().

-----

This looks like it matches the approach I suggested.



================
Comment at: clang/lib/CodeGen/CGCall.cpp:2338
+  auto AddArm64ECX86Sign = [this](QualType Ty, llvm::AttrBuilder &Attrs) {
+    if (Ty->isRecordType() || Ty->isArrayType()) {
+      unsigned SizeInBits = getContext().getTypeSize(Ty.getTypePtr());
----------------
`Ty->isArrayType()` will never be true; type promotion rules don't allow specifying an array as a function argument.


================
Comment at: clang/lib/CodeGen/CGCall.cpp:2340
+      unsigned SizeInBits = getContext().getTypeSize(Ty.getTypePtr());
+      unsigned SizeInBytes = (SizeInBits + 7) / 8;
+      Attrs.addArmECArgX86SignAttr(SizeInBytes);
----------------
Please use getTypeSizeInChars().


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

https://reviews.llvm.org/D132926