<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Wed, Apr 1, 2015 at 11:22 AM, Ahmed Bougacha <span dir="ltr"><<a href="mailto:ahmed.bougacha@gmail.com" target="_blank">ahmed.bougacha@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">On Wed, Apr 1, 2015 at 10:59 AM, Pirama Arumuga Nainar<br>
<<a href="mailto:pirama@google.com">pirama@google.com</a>> wrote:<br>
><br>
><br>
> On Wed, Apr 1, 2015 at 10:35 AM, Ahmed Bougacha <<a href="mailto:ahmed.bougacha@gmail.com">ahmed.bougacha@gmail.com</a>><br>
> wrote:<br>
>><br>
>> Awesome, I wanted to do this next, thanks for working on this =)<br>
>><br>
>> I'll look into this closer, but a couple of very high level<br>
>> not-really-questions first:<br>
>><br>
>> - what about other types?  I just removed the generic f32->f64 promotion a<br>
>> few days ago (in anticipation for such a PromoteFloat legalizer).  I'm not<br>
>> sure we can test that in-tree, so there's probably not much point in<br>
>> worrying about this.<br>
><br>
><br>
> I've tried to keep promotion as generic as possible.  As long as the<br>
> PromoteFloat typeaction is set and TransformToType is set correctly, it<br>
> should be able to handle f32->f64.  I haven't tested it though.<br>
><br>
>> - might I ask: what's your use case?  In particular, do you intend to<br>
>> change clang in any way?<br>
><br>
><br>
> I am not planning to change Clang.  The use case is to handle bitcode with<br>
> Clang run with NativeHalfType and HalfArgsAndReturns arguments enabled, but<br>
> the target/backend doesn't have support for f16.<br>
<br>
</span>Alright, then why a command-line option anyway?  On any IR with half,<br>
but targeting platforms without it, we'll just crash.  For the few<br>
nodes where we don't (load/store), we soften to i16 here as well, so<br>
there should be no difference.<br>
<br>
Why not just always promote when f16 isn't legal but f32 is?<br></blockquote><div><br></div><div>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.</div><div><br></div><div>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.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<span class="HOEnZb"><font color="#888888"><br>
-Ahmed<br>
</font></span><div class="HOEnZb"><div class="h5"><br>
>><br>
>><br>
>> In the meantime, you might be interested in <a href="http://reviews.llvm.org/D8648" target="_blank">http://reviews.llvm.org/D8648</a><br>
>> for tests, as I tried to cover as many IR/generic ISD opcodes as possible.<br>
>> The patch itself just focuses on ops promotion on AArch64 (where half is<br>
>> legal).<br>
><br>
><br>
> I did peek into your patch when I wrote my tests :)  I'll refer to them when<br>
> I write target-specific tests.<br>
>><br>
>><br>
>> -Ahmed<br>
>><br>
>><br>
>> <a href="http://reviews.llvm.org/D8755" target="_blank">http://reviews.llvm.org/D8755</a><br>
>><br>
>> EMAIL PREFERENCES<br>
>>   <a href="http://reviews.llvm.org/settings/panel/emailpreferences/" target="_blank">http://reviews.llvm.org/settings/panel/emailpreferences/</a><br>
>><br>
>><br>
><br>
</div></div></blockquote></div><br></div></div>