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

Thu Aug 4 18:58:19 PDT 2022

rjmccall added a comment.

In D124435#3701163 <https://reviews.llvm.org/D124435#3701163>, @jyknight wrote:

> In D124435#3697259 <https://reviews.llvm.org/D124435#3697259>, @rjmccall wrote:
>
>> I know what you're saying, but I don't think it matches any model of how programmers use command line flags.  You're imagining that a programmer sits down and considers all of their flags deeply and holistically before touching any of them, and that's just not how these things go in actual build systems.  Flags have a tendency to be set in separate places and therefore to drift.  Someone setting this option is doing it because they have a concrete compatibility need, and the flag going from meaning "be compatible with GCC" to meaning "be compatible with old Clangs" based on the value of a separate flag is surprising and likely to cause bugs.
>
> Funny, my argument is based precisely on the fact that people _don't_ understand deeply what things are doing, and that the original proposal is likely to trigger bugs via mistakes in its usage!
>
> The default (`-mconservative-small-integer-abi`) is "be compatible with all Clang and GCC versions" -- it "just works". (I think everyone agrees upon that being the default.)
>
> So, the use-case for `-mconservative-small-integer-abi` is not a need for compatibility but rather a need for performance/code-size overriding cross-compiler compatibility concerns. In that case, the user better be able to answer "what other compiler/version do you need to be compatible with, if any?" -- and set -fclang-abi-compat flag appropriately based on that answer.
>
> That seems much easier thing to explain to a user than the details of where exactly extension may/must happen in the call ABI.

I'm not arguing that we shouldn't work `-fclang-abi-compat` into this; I think that is a very good idea.  And for many people, that'll be all they need.  My concern is about the people who do add `-mno-conservative-small-integer-abi` to their builds.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124435