[PATCH][X86] Add ISel patterns to select 'fp32_to_fp16' and 'fp16_to_fp32' dag nodes.
Jim Grosbach
grosbach at apple.com
Thu Jul 3 11:11:36 PDT 2014
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>
More information about the llvm-commits
mailing list