[PATCH] Add support to promote f16 to f32

[Pirama Arumuga Nainar]

On Wed, Apr 1, 2015 at 11:22 AM, Ahmed Bougacha <ahmed.bougacha at gmail.com>
wrote:

> On Wed, Apr 1, 2015 at 10:59 AM, Pirama Arumuga Nainar
> <pirama at google.com> wrote:
> >
> >
> > On Wed, Apr 1, 2015 at 10:35 AM, Ahmed Bougacha <
> ahmed.bougacha at gmail.com>
> > wrote:
> >>
> >> Awesome, I wanted to do this next, thanks for working on this =)
> >>
> >> I'll look into this closer, but a couple of very high level
> >> not-really-questions first:
> >>
> >> - what about other types?  I just removed the generic f32->f64
> promotion a
> >> few days ago (in anticipation for such a PromoteFloat legalizer).  I'm
> not
> >> sure we can test that in-tree, so there's probably not much point in
> >> worrying about this.
> >
> >
> > I've tried to keep promotion as generic as possible.  As long as the
> > PromoteFloat typeaction is set and TransformToType is set correctly, it
> > should be able to handle f32->f64.  I haven't tested it though.
> >
> >> - might I ask: what's your use case?  In particular, do you intend to
> >> change clang in any way?
> >
> >
> > I am not planning to change Clang.  The use case is to handle bitcode
> with
> > Clang run with NativeHalfType and HalfArgsAndReturns arguments enabled,
> but
> > the target/backend doesn't have support for f16.
>
> Alright, then why a command-line option anyway?  On any IR with half,
> but targeting platforms without it, we'll just crash.  For the few
> nodes where we don't (load/store), we soften to i16 here as well, so
> there should be no difference.
>
> Why not just always promote when f16 isn't legal but f32 is?
>

For one, I didn't want to do this non-intrusively and not affect others
relying on old behavior.  But as you mentioned, the old behavior is to
crash, so this'd be better as the default.

The second reason is that I am unsure if keeping intermediate results as
f32 will break someone's precision assumptions.  I am more than ok to make
Promotion the default when f16 is illegal, but f32 is.


>
> -Ahmed
>
> >>
> >>
> >> In the meantime, you might be interested in
> http://reviews.llvm.org/D8648
> >> for tests, as I tried to cover as many IR/generic ISD opcodes as
> possible.
> >> The patch itself just focuses on ops promotion on AArch64 (where half is
> >> legal).
> >
> >
> > I did peek into your patch when I wrote my tests :)  I'll refer to them
> when
> > I write target-specific tests.
> >>
> >>
> >> -Ahmed
> >>
> >>
> >> http://reviews.llvm.org/D8755
> >>
> >> EMAIL PREFERENCES
> >>   http://reviews.llvm.org/settings/panel/emailpreferences/
> >>
> >>
> >
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150401/5e7340fa/attachment.html>