[PATCH][X86] Add ISel patterns to select 'fp32_to_fp16' and 'fp16_to_fp32' dag nodes.

[Andrea Di Biagio]

Thanks Jim!

Committed revision 212293.

On Thu, Jul 3, 2014 at 7:11 PM, Jim Grosbach <grosbach at apple.com> wrote:
> LGTM. Thanks!
>
>> On Jun 27, 2014, at 2:52 PM, Andrea Di Biagio <andrea.dibiagio at gmail.com> wrote:
>>
>> ping.
>>
>>
>> Thanks,
>> Andrea
>>
>> On Sat, Jun 21, 2014 at 1:25 AM, Andrea Di Biagio
>> <andrea.dibiagio at gmail.com> wrote:
>>> Hi Jim,
>>> sorry for the multiple emails...
>>>
>>> I think I now understand the problem in the code.
>>>
>>> This line:
>>>
>>> +  if (!TM.Options.UseSoftFloat && !Subtarget->hasF16C()) {
>>>
>>> should have been instead:
>>>
>>> +  if (TM.Options.UseSoftFloat || !Subtarget->hasF16C()) {
>>>
>>> Basically, the idea is:
>>> if -soft-float=1, then we always expand the float conversions into
>>> calls to the runtime library.
>>> Otherwise, we check for feature F16C; if F16C is not available, then we expand
>>> the float conversions into libcalls.
>>>
>>> I attached a new version of the patch that fixes the problem and adds two new
>>> tests in cvt16.ll to verify that we always emit libcalls when flag
>>> -soft-float=1 is specified.
>>>
>>> Please let me know what you think.
>>>
>>> Thanks!
>>> Andrea
>>>
>>> On Fri, Jun 20, 2014 at 11:59 PM, Andrea Di Biagio
>>> <andrea.dibiagio at gmail.com> wrote:
>>>> Do you think it would it be ok if I just remove the check for soft-float?
>>>>
>>>>
>>>> On Fri, Jun 20, 2014 at 11:29 PM, Andrea Di Biagio
>>>> <andrea.dibiagio at gmail.com> wrote:
>>>>> Hi Jim,
>>>>>
>>>>> On Fri, Jun 20, 2014 at 9:32 PM, Jim Grosbach <grosbach at apple.com> wrote:
>>>>>> I agree calling library helpers is the correct fallback here.
>>>>>>
>>>>>> What’s the behavior with soft-float? Specifically,
>>>>>>
>>>>>> +  if (!TM.Options.UseSoftFloat && !Subtarget->hasF16C()) {
>>>>>
>>>>> I have just run some tests (using the test-case from this patch)
>>>>> with/without flag -float-abi=soft/hard.
>>>>> The presence/absence of that flag doesn't seem to affect the output.
>>>>>
>>>>> My originally idea was to generate library calls for
>>>>> float-to-half-float conversions if 'soft' float-abi was specified.
>>>>> I thought that the presence of that flag would have affected the
>>>>> codegen for those dag nodes. But apparently I was wrong.
>>>>>
>>>>>>
>>>>>> -Jim
>>>>>>
>>>>>>> On Jun 19, 2014, at 6:53 AM, Andrea Di Biagio <andrea.dibiagio at gmail.com> wrote:
>>>>>>>
>>>>>>> Hi,
>>>>>>>
>>>>>>> this patch:
>>>>>>> 1) Adds tablegen patterns to select F16C float-to-half-float
>>>>>>> conversion instructions (and vice versa) from 'f32_to_f16' (and
>>>>>>> 'f16_to_f32') dag nodes;
>>>>>>>
>>>>>>> 2) Teaches the backend how to emit compiler runtime library call to
>>>>>>> __gnu_f2h_ieee and __gnu_h2f_ieee if there is no F16C support.
>>>>>>>
>>>>>>> About point 2.
>>>>>>> I am not sure if there is a better ways to fix/workaround this
>>>>>>> problem. In general, my opinion is that the backend shouldn't raise a
>>>>>>> error if we don't have F16C and the DAG contains
>>>>>>> 'f32_to_f16/f16_to_f32' nodes.
>>>>>>>
>>>>>>> The compiler already knows about the existence of libcalls
>>>>>>> '__gnu_f2h_ieee' and '__gnu_h2f_ieee2' (see TargetLoweringBase.cpp).
>>>>>>> So, my idea (if you agree) is to generate runtime calls instead of
>>>>>>> returning an ISel error.
>>>>>>> However, this could result later on in a linker error if the runtime
>>>>>>> library doesn't define '__gnu_f2h_ieee' and '__gnu_h2f_ieee'.
>>>>>>>
>>>>>>> Please let me know what you think.
>>>>>>>
>>>>>>> Thanks!
>>>>>>> Andrea Di Biagio
>>>>>>> <patch-f16c-conv.diff>
>>>>>>
>> <patchv2-f16c-conv.diff>
>