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

[Michael Liao]

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.

Yours
- Michael


> 
> -Eli