[PATCH] D122963: [X86] Extend the integer parameter if the function isn't local linked

[Reid Kleckner via Phabricator via llvm-commits]

rnk requested changes to this revision.
rnk added a comment.
This revision now requires changes to proceed.

In D122963#3456350 <https://reviews.llvm.org/D122963#3456350>, @LuoYuanke wrote:

> I think we all agree that this is a bug. Given there is no better solution for it, I'd like check in this patch first. We can revert the patch when there is better solution proposed.

I don't think we've reached agreement that this is a bug. I am concerned that removing these extension assert nodes will result in performance regressions. Here's the summary table from the RFC:

> clang: caller do the zero/sign extension while callee do nothing.
> gcc: both caller and callee do the zero/sign extension.
> icc: callee do the zero/sign extension while caller do nothong.

The way I read that, GCC and Clang are ABI compatible with each other.

The incompatibility exists between ICC and Clang/LLVM. ICC does not extend parameters, but Clang/LLVM assumes they will have been extended in the caller.

---

I think in cases where we disagree and are blocking each other, that's when we need to resort to flags to make forward progress. What do you think about adding a cl::opt for this feature? At the very least, this will allow folks to do their own performance testing, even if we ultimately default to GCC's behavior. I see the OpenVMS folks <https://discourse.llvm.org/t/always-extend-the-integer-parameters-of-callee/61319/9?u=rnk> have a setting for this as well ("assume clean parameters").

Finally, maybe this should be discussed in the x86_64 psABI issue tracker <https://gitlab.com/x86-psABIs/x86-64-ABI>. At the very least, the psabi docs could be clearer.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122963