[PATCH] D122594: [ArgPromotion] Update min-legal-vector-width when do promotion

[Phoebe Wang via Phabricator via llvm-commits]

pengfei added a subscriber: andrew.w.kaylor.
pengfei added a comment.

In D122594#3417080 <https://reviews.llvm.org/D122594#3417080>, @craig.topper wrote:

> In D122594#3417023 <https://reviews.llvm.org/D122594#3417023>, @craig.topper wrote:
>
>> Have you debugged why areFunctionArgsABICompatible isn't preventing this properly?
>
> Is that really an inliner problem?

No. I didn't investigate it much, but I think it's not an inliner problem. Increasing "min-legal-vector-width" of a function should always be safe if we never touch the arguments. And we should always increasing it to make sure the correctness of the inlined functions. Here the ArgumentPromotion changes arguments without setting the correct "min-legal-vector-width" that looks a problem to me.

> Or are we assuming that if there is a vector argument, then min-legal-vector-width should always be >= the size of the vector argument?

Yes and no. We can't always set the "min-legal-vector-width" to the size of  the largest vector argument. From the perspective of ABI, the "min-legal-vector-width" is the largest vector that meets the current ABI.
But when the ABI doesn't matter, we are free to set it to the size of  the largest vector argument.
ArgumentPromotion is a good example, where we can break ABI due to we do it for internal functions only. E.g., https://godbolt.org/z/dabbdcYPb
As you can see, `__m1024` should be passed by memory according to ABI. But we will spilt it if we build a 1024 bit vector in arguments.

As @andrew.w.kaylor has pointer on https://discourse.llvm.org/t/vector-abi-and-min-legal-vector-width/60615, middle end usually doesn't have the ability to check the ABI and decides the proper vector or memory thing. So I think it's practicable to use arbitary vector type in arguments in middle end (as long as they never link externally, i.e., ABI doesn't matter) and set "min-legal-vector-width" to the size of  the largest vector. It also conforms to the behavior with no "min-legal-vector-width", which we will set it to the largest integer.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122594