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

[Zahira Ammarguellat via Phabricator via llvm-commits]

zahiraam added a comment.

In D113107#3736325 <https://reviews.llvm.org/D113107#3736325>, @rjmccall wrote:

> In D113107#3736312 <https://reviews.llvm.org/D113107#3736312>, @zahiraam wrote:
>
>> This is a reduced test case from the codegen/complex-strictfp.c
>>
>> _Complex double g1, g2;
>> double D;
>>
>> void foo(void) {
>>
>>   g1 = g1 + D;
>>
>> }
>>
>> The issue is that we are calling in VisitBinAssign the function  EmitUnpromotion (since promotionTy is null). This creates 2 constrained (-ffp-exception-behavior=maytrap used) fptrunc instructions. The verifier for Intrinsic::experimental_constrained_fptrunc fails at this check:
>> https://github.com/intel/llvm/blob/sycl/llvm/lib/IR/Verifier.cpp#L5885
>> Is the call for EmitUnpromotion at the right place? Or should VisitBinAssign be treated the same way than the operator of HANDLEBINOP? Since in this case we are in the unpromoted path, we don't really need to add those fptrunc? Any thoughts?
>
> If `VisitBinAssign` isn't opting to promoted emission (by calling `EmitPromoted...`), it shouldn't have to call `EmitUnpromotion`; it should be able to rely on getting appropriate values for the type of its operands.  That's what I mean by a strong postcondition.  So yeah, if you added a call to `EmitUnpromotion` there as a workaround at some point, you should take it out.  If that breaks tests, it's because some emitter is leaking promoted values out of the normal emission path, and we need to fix that emitter.

Fixed codegen issue and LIT test issue mentioned by @rjmccall.
I need second pair of eyes to check the resulting IR, @pengfei  can you please help with that?
Do we want to add the promoted path for VistiBinAssign in this patch or can we add it in a subsequent patch? 
Thanks.


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

https://reviews.llvm.org/D113107