pengfei added a comment. > 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). There are two aspects I'd argue: 1. According to LanguageExtensions <https://clang.llvm.org/docs/LanguageExtensions.html#half-precision-floating-point>, a target needs to define ABI and the backend (at least) needs to provide the arguments passing/returning for `_Float16`. As we talked above, we cannot assume the ABI of `_Float16` is the same as `float` and promote to it in the front-end. So we cannot make it work without any backend change. And if we have to change backend, it's not a big deal to support lowering operations at the same time. We have target independent framework to help with that; 2. Promotion then unpromotion for each expression is cumbersome, inefficient and it makes it rather complicated for further optimization as you have described above. WDYT? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D113107/new/ https://reviews.llvm.org/D113107 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
