[PATCH] D86476: [AIX] Add support for non var_arg extended vector ABI calling convention on AIX

[Zarko Todorovski via Phabricator via llvm-commits]

ZarkoCA marked an inline comment as done.
ZarkoCA added inline comments.


================
Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.cpp:6925
 
-  assert((!ValVT.isInteger() ||
-          (ValVT.getSizeInBits() <= RegVT.getSizeInBits())) &&
-         "Integer argument exceeds register size: should have been legalized");
+  if (ValVT.isInteger() && !ValVT.isVector())
+    assert(
----------------
Xiangling_L wrote:
> Just want to confirm that this would not lead to a warning when compiled with assertion off?
I think so, but I wasn't looking to mess with that assert only to avoid it when using vectors.  


================
Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.cpp:6948
+
   if (ArgFlags.isByVal()) {
     if (ArgFlags.getNonZeroByValAlign() > PtrAlign)
----------------
Xiangling_L wrote:
> If I understand correctly, `isByVal()` section is for aggregate parameter types only? Can we add some comments or even better an assertion here to indicate that?
Yes, that sections only handles arguments that have the ByVal flag.  

I think adding comments or an assertion is not related to this patch.  We would need a separate review to handle that. 


================
Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.cpp:7048
+  case MVT::v2f64:
+  case MVT::v2i64:
+  case MVT::v1i128: {
----------------
Xiangling_L wrote:
> Are we using `v2i64` to support `vector long`? As ABI says, `the vector type with the long keyword are deprecated`. Should we emit an error for this type instead?
Good catch, thanks. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86476