[llvm-commits] [Fwd: [PATCH] Add custom UINT_TO_FP lowering from v2i32 to v2f32 in 32-bit mode]

[Eli Friedman]

On Tue, Oct 23, 2012 at 6:07 PM, Michael Liao <michael.liao at intel.com> wrote:
> On Tue, 2012-10-23 at 17:52 -0700, Eli Friedman wrote:
>> On Tue, Oct 23, 2012 at 5:38 PM, Michael Liao <michael.liao at intel.com> wrote:
>> > On Tue, 2012-10-23 at 17:35 -0700, Eli Friedman wrote:
>> >> On Tue, Oct 23, 2012 at 5:24 PM, Michael Liao <michael.liao at intel.com> wrote:
>> >> > On Tue, 2012-10-23 at 17:17 -0700, Eli Friedman wrote:
>> >> >> On Tue, Oct 23, 2012 at 5:01 PM, Michael Liao <michael.liao at intel.com> wrote:
>> >> >> > Sent to wrong list. What am I doing? - Michael
>> >> >> >
>> >> >> >
>> >> >> > ---------- Forwarded message ----------
>> >> >> > From: Michael Liao <michael.liao at intel.com>
>> >> >> > To: <llvmdev at cs.uiuc.edu>
>> >> >> > Cc:
>> >> >> > Date: Tue, 23 Oct 2012 16:51:54 -0700
>> >> >> > Subject: [PATCH] Add custom UINT_TO_FP lowering from v2i32 to v2f32 in 32-bit mode
>> >> >> > Hi
>> >>
>> >> Okay; please separate into two patches when you commit, then.
>> >>
>> >> I think the DAGCombine might apply too broadly.  For example, turning
>> >> BUILD_VECTOR(UINTTOFP(i32 %x),undef,undef,undef) into
>> >> UINTTOFP(BUILD_VECTOR(i32 %x,undef,undef,undef)) is a bad idea on
>> >> x86-64.
>> >
>> > The added DAG combine only works when target claims the UINT_TO_FP on
>> > the destination type is supported. In 64-bit mode, UINT_TO_FP is claimed
>> > to be 'Expand' and this transformation won't be applied.
>>
>> Okay... so then this won't transform BUILD_VECTOR(UINTTOFP(i32
>> %x),UINTTOFP(i32 %y),UINTTOFP(i32 %z),UINTTOFP(i32 %w))?
>
> Yeah, this patch so far doesn't address that yet. If we decide to
> support that, it's most likely to be supported in 32-bit mode only as
> 64-bit mode has 64-bit GPR and the overhead of converting u32 to f32 is
> quite low.
>
> BTW, please check attached patches after splitting.
>
> * 0001-Add-custom-UINT_TO_FP-conversion-from-v2i32-to-v2f32.patch, this
> patch is revised after your comments on result type checking.
>
> * 0002-Teach-DAG-combine-to-transform-build_vec-fptoXi-to-f.patch, this
> patch adds minor checking to address your concern, i.e. if that vector
> has only 1 element defined, the transformation will bail out as the
> vectorized one won't be better than the scalar one.

Looks good; thanks.

-Eli