[PATCH] D122104: [X86][regcall] Support passing / returning structures

[Phoebe Wang via Phabricator via cfe-commits]

pengfei added inline comments.


================
Comment at: clang/lib/CodeGen/CGCall.cpp:5238
+  for (unsigned i = 0; i < IRCallArgs.size(); ++i)
+    LargestVectorWidth = std::max(LargestVectorWidth,
+                                  getMaxVectorWidth(IRCallArgs[i]->getType()));
----------------
pengfei wrote:
> LuoYuanke wrote:
> > Does this also affect other calling convention besides fastcall?
> I don't think so. The change here adds for the missing cases like `[2 x <4 x double>]` or `{ <2 x double>, <4 x double> }` which should also set `min-legal-width-width` to the maximum of the vector length.
> There're several reasons why other calling convention won't be affected.
> 1. If a target has ability to pass arguments like `[2 x <4 x double>]`, it must have the ability for `<4 x double>` and have set `min-legal-width-width` to 256 when passing it. So it makes more sense to set `min-legal-width-width` to 256 for `[2 x <4 x double>]` rather than keeping it as 0;
> 2. AFAIK, targets other than X86 simply ignore `min-legal-width-width`. So the change won't affect them;
> 3. On x86, calling conventions other than regcall don't allow arguments size larger than 512, see `if (!IsRegCall && Size > 512)`. They will be turned into pointers, so they won't be affected by this change;
> 4. For arguments size no larger than 512 and only contain single vector element, we have already turned them into pure vectors. So they have already set `min-legal-width-width` to the correct value;
> 5. For arguments have more then one vector elements. Clang has bug which doesn't match with GCC and ICC. I have filed a bug here https://github.com/llvm/llvm-project/issues/54582
> 6. Thus, only regcall can generate arguments type like `[2 x <4 x double>]` on X86. So only it will be affected by this.
Update for bullet 5. The handling for multi vector elements is correct. It's also passed / returned by memory. So it's still not affected by this change.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122104