[PATCH] D113107: Support of expression granularity for _Float16.

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

In D113107#3744505 <https://reviews.llvm.org/D113107#3744505>, @pengfei wrote:

>> I'm not sure what optimization you mean. Because the ABI returns 16-bit and 32-bit FP values differently, there really isn't a way that we can return a value without going through a truncation/extension cycle.
>
> I explained it to Zahira offline. I forgot we have different expectation for the patch, thus we were talking different optimization to each other. I expected each backend has the ability to lower half operations. So I emphasized not to do the promotion or eliminate unnecessary promotion at the begining. While I see your point, we can only eliminate or combine unpromotion to the following promotion, so that we don't leave half operations to backends.
>
>> There's potential to eliminate those with IPO, but we should definitely leave that for a different patch, for two reasons:
>
> I agree with you, IPO is the only chance to do elimination.
>
>> Somehow we've taken a huge step back on unpromotion, and I'm worried you're now doing the exact thing I didn't want us doing and forcing all the downstream clients to handle the possibility of a promoted result.
>
> However, I am still not persuaded we need to consider the backends not supporting half operations (if I understand your downstream clients correctly).

Oh, no, LLVM is not what I mean by downstream clients.  That comment was meant for Zahira and is about a purely internal matter to IRGen; the downstream clients I meant are just the call sites within IRGen of e.g. EmitScalarExpr.  We've been having some kind of persistent disconnect throughout this review about IRGen's postconditions on expression emission.  Unfortunately, IRGen doesn't have good assertions in place for its postconditions, so breaking the postconditions can silently turn into miscompiles instead of assertion failures.  For example, Clang's IR emission for `return` statements has some well-meaning code that tries to handle minor type mismatches between the ABI-dictated IR return type and the IR type that expression-emission produces by coercing the value through memory.  Unfortunately, that code is also triggered when expression emission just has a bug and returns the wrong type, and so you can see a bunch of code in the tests that's doing stuff like storing a pair of `float`s into memory and then loading a pair of `half`s out, which is obviously incorrect.  None of this has anything to do with our conversation about avoiding fptrunc/fpext pairs over return boundaries.


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

https://reviews.llvm.org/D113107