[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 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
>> >> >
>> >> > As 32-bit mode doesn't have 64-bit GPR, the sequence converting v2i32 to
>> >> > v2f32 is quite inefficient in 32-bit mode. This patch adds the custom
>> >> > lowering in 32-bit mode. In addition, it teaches DAG combine to
>> >> > transform (build_vec (Xint2fp x) (Xint2fp y) ..) to (Xint2fp (build_vec
>> >> > x y)) to reduce the strength on FP conversion unit.
>> >> >
>> >> > Thanks for your review
>> >>
>> >> +  case ISD::UINT_TO_FP: {
>> >> +    if (N->getOperand(0).getValueType() != MVT::v2i32)
>> >> +      return;
>> >>
>> >> Please check the result type as well, to make it clear what this code is doing.
>> >
>> > Sure, I will add extra checking to not do something wrong.
>> >
>> >>
>> >> Is there any way you can come up with testcases for each individual
>> >> part of this patch?  It looks like you're making three different
>> >> changes at the same time.
>> >
>> > This patch just adds two major changes. The patch to DAG combine depends
>> > on the change adding in X86 backend. The test case include both tests: 1
>> > is trivial converting from v2i32 to v2f32; the other is the case where
>> > the change to DAG combine is to be applied.
>>
>> 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))?

-Eli