[PATCH] D112942: target ABI: improve call parameters extensions handling

[Jonas Paulsson via Phabricator via llvm-commits]

jonpa added a comment.

In D112942#3523792 <https://reviews.llvm.org/D112942#3523792>, @efriedma wrote:

> Not sure how much I like the rule about "externally visible functions"... I mean, I guess restricting the checking to external functions avoids triggering on a bunch of cases that would be difficult to handle, but it doesn't really seem self-consistent.

Well, if we decide to in llvm *not* do the optimization of removing an extension of an incoming argument (which the ABI says is legal), our only concern is when calling or returning a value from an externally visible function. I think this is a legacy issue and not important for performance, so acting on the attribute when compiling is not something we plan to do (at least on SystemZ).

> How about this: we introduce NoExt, but in LangRef we just say "If an integer argument to a function is not marked signext/zeroext/noext, the kind of extension is used is target-specific. Some targets may generate less efficient code if the kind of extension is not explicitly specified."  This is basically what you're doing now: follow the attribute, and sign-extend in the caller if there's no attribute.  But there isn't an implication that generating calls without any extension attributes is invalid on all targets.

I consider the "default extension" to be related but not really the same idea: it merely serves as a "better than nothing" solution, but I think that may be removed in the future, and the verification instead is on by default. Actually, this could be done for SystemZ now at least as things look "green" now. Note that this default extension could be wrong (use SExt instead of ZExt on a very big unsigned value).

How about:
"If an integer argument to a function is not marked signext/zeroext/noext, the kind of extension used is target-specific. Some targets depend for correctness the kind of extension to be explicitly specified."

> For the lint in the backend, instead of checking whether the callee is an external symbol, maybe check whether the calling convention is "C" (as opposed to "fast").

Do all externally visible functions always have the "C" calling convention?

Should the VerifyIntegerArg() and CheckNarrowIntegerArgs() remain in SystemZ or should I move them somewhere and let any target use them as desired? I think the default extension could be done if desired entirely in the target backend, or?


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

https://reviews.llvm.org/D112942