[PATCH] D124435: [X86] Always extend the integer parameters in callee

[James Y Knight via Phabricator via cfe-commits]

jyknight added a comment.

Thinking more about the naming issue, I think the real issue is that the proposed options are an implementation detail, not what we should expose to users. I realize that the non-boolean option was proposed by rjmmccall before, but I'd suggest that we go back to something more like your original boolean option. I believe that the discussion which resulted in the new option, was due to confusion around what "not conservative" should mean -- and that's just because the implementation did not honor `fclang-abi-compat`.

The main problem I have with the current proposal is that it's both confusing to describe (I still can't really understand the docs), and it invites trouble: if a user is trying to improve performance and specifies `-mextend-small-integers=none`, that will help on Linux but break the binary on Apple. There's really no reason to even offer "none" on apple.

So, let's provide the ability to adjust behavior according to the goals a user may have. The behaviors I think we need to expose are:

1. (Default) Be compatible both with previous Clang ABI, and with GCC/ICC/new-Clang ABI.
2. Be compatible with previous Clang ABI, don't care about compatibility with standard ABI.
3. Be compatible with standard ABI, don't care about compatibility with previous Clang ABI.

So, for that purpose, I suggest we should use the existing `-fclang-abi-compat` option, plus a single on/off switch for whether to be conservatively-compatible (which I've renamed `-mconservative-small-integer-abi` but I don't feel strongly about that if you wanted to go back to `-mconservative-extend`)

Then, to achieve the 3 goals above, you can specify:

1. No extra args (which implies the defaults of `-fclang-abi-compat=16 -mconservative-small-integer-abi`)
2. `-fclang-abi-compat=15 -mno-conservative-small-integer-abi`
3. `-mno-conservative-small-integer-abi `

On most SysV platforms, with `-mno-conservative-small-integer-abi`, abi-compat=15 will use Direct, and abi-compat=16 will use Extend. With `-mconservative-small-integer-abi`, both will use ConservativeExtend mode. On Apple/Playstation platforms, this ABI doesn't change between 15/16, so it should always use Extend regardless of either of the options. (That is, `-mconservative-small-integer-abi` is a no-op on those platforms.)

Then we can go back to the previous description from rjmmcall, with some minor adjustments to describe the interaction with -fclang-abi-compat:

  -mconservative-small-integer-abi
  
  In the past, on certain targets, Clang passed promotable integer arguments
  (arguments with types smaller than int) using a parameter convention in which
  the caller was assumed to have sign-extended or zero-extended the argument to
  the width of an int. This convention did not conform to the documented ABI for
  these targets, which does not require the caller to perform this
  extension. Under `-fclang-abi-compat=16`, Clang no longer assumes that callers
  perform this extension, but for compatibility with code compiled by previous
  releases of Clang, Clang defaults to still extending the argument in the
  caller.  `-mno-conservative-small-integer-abi` disables this, which may
  improve performance and code size if compatibility across Clang versions is
  not required.
  
  This option is a no-op for targets which did not have an ABI change (Apple,
  Windows, and PlayStation).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124435