[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: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.

-Eli