[llvm-commits] [llvm] r168029 - /llvm/trunk/lib/Target/ARM/ARMISelLowering.cpp

Thu Nov 15 19:01:24 PST 2012

Task captured in PR14359: http://llvm.org/bugs/show_bug.cgi?id=14359

-- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted
by The Linux Foundation

From: llvm-commits-bounces at cs.uiuc.edu
[mailto:llvm-commits-bounces at cs.uiuc.edu] On Behalf Of David Peixotto
Sent: Thursday, November 15, 2012 4:03 PM
To: 'Craig Topper'
Cc: llvm-commits at cs.uiuc.edu
Subject: Re: [llvm-commits] [llvm] r168029 -
/llvm/trunk/lib/Target/ARM/ARMISelLowering.cpp

Ah, I see.

Yes, we can use a Q register to do the vabs in one instruction. The code we
should generate would be something like:

        vmov    d1, r2, r3

        vmov    d0, r0, r1

        vabs.f32        q1, q0

        vmov    r0, r1, d2

        vmov    r2, r3, d3

Thanks for the small test case. I'm going to file a bug so we remember to
fix it.

-- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted
by The Linux Foundation

From: Craig Topper [mailto:craig.topper at gmail.com] 
Sent: Thursday, November 15, 2012 3:22 PM
To: David Peixotto
Cc: llvm-commits at cs.uiuc.edu
Subject: Re: [llvm-commits] [llvm] r168029 -
/llvm/trunk/lib/Target/ARM/ARMISelLowering.cpp

I did an exhaustive run of all of the floating point intrinsics against
several targets and found this failure.

After my change this is the generated code which does fabs.f32 on each
element. Is there a vector fabs that could be used instead?

        vmov    d1, r2, r3

        vmov    d0, r0, r1

        vabs.f32        s7, s3

        vabs.f32        s6, s2

        vabs.f32        s5, s1

        vabs.f32        s4, s0

        vmov    r0, r1, d2

        vmov    r2, r3, d3

This is the test code

define <4 x float> @test(<4 x float> %a) {

  %foo = call <4 x float> @llvm.fabs.v4f32(<4 x float> %a)

  ret <4 x float> %foo

}

declare <4 x float> @llvm.fabs.v4f32(<4 x float> %a)

On Thu, Nov 15, 2012 at 2:32 PM, David Peixotto <dpeixott at codeaurora.org>
wrote:

Was there a bug filed for the original failure?

I don't think it is a good idea to quietly silence failures like these by
inserting a work around. A failure in instruction selection is an important
signal that we are potentially missing a pattern. We should address the root
cause by adding a pattern to select the correct instruction rather than just
inserting a work around. I expect we will see more failures like this as the
vectorizer stresses the back ends and we need to stay on top of them so that
we generate good quality code.

Do you perhaps have a small test case we can use to file a bug as a reminder
to investigate this issue?

-- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted
by The Linux Foundation

From: Craig Topper [mailto:craig.topper at gmail.com] 
Sent: Thursday, November 15, 2012 11:35 AM
To: David Peixotto
Cc: llvm-commits at cs.uiuc.edu
Subject: Re: [llvm-commits] [llvm] r168029 -
/llvm/trunk/lib/Target/ARM/ARMISelLowering.cpp

My primary goal was to keep instruction selection from failing. Without the
change I made, FABS of v4f32 just didn't match anything and failed. Someone
familiar with ARM could maybe do something better.

On Thu, Nov 15, 2012 at 9:52 AM, David Peixotto <dpeixott at codeaurora.org>
wrote:

Hi Craig,

I see the change was reverted for FNEG, but what about FABS? Why does FABS
need to be marked as expand here? There is a VABS.f32 instruction that
operates on Q registers that should work for a v4f32 type. Will that
instruction be generated when it is marked expand?

Thanks,
David

-- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted
by The Linux Foundation

> -----Original Message-----
> From: llvm-commits-bounces at cs.uiuc.edu [mailto:llvm-commits-
> bounces at cs.uiuc.edu] On Behalf Of Craig Topper
> Sent: Thursday, November 15, 2012 12:06 AM
> To: llvm-commits at cs.uiuc.edu
> Subject: [llvm-commits] [llvm] r168029 -
> /llvm/trunk/lib/Target/ARM/ARMISelLowering.cpp
>
> Author: ctopper
> Date: Thu Nov 15 02:06:12 2012
> New Revision: 168029
>
> URL: http://llvm.org/viewvc/llvm-project?rev=168029
<http://llvm.org/viewvc/llvm-project?rev=168029&view=rev> &view=rev
> Log:
> Make FNEG and FABS of v4f32 Expand.
>
> Modified:
>     llvm/trunk/lib/Target/ARM/ARMISelLowering.cpp
>
> Modified: llvm/trunk/lib/Target/ARM/ARMISelLowering.cpp
> URL: http://llvm.org/viewvc/llvm-
> project/llvm/trunk/lib/Target/ARM/ARMISelLowering.cpp?rev=168029&r1=1
> 68028&r2=168029&view=diff
> ==========================================================
> ====================
> --- llvm/trunk/lib/Target/ARM/ARMISelLowering.cpp (original)
> +++ llvm/trunk/lib/Target/ARM/ARMISelLowering.cpp Thu Nov 15 02:06:12
> 2012
> @@ -505,6 +505,8 @@
>      setOperationAction(ISD::FNEARBYINT, MVT::v2f64, Expand);
>      setOperationAction(ISD::FFLOOR, MVT::v2f64, Expand);
>
> +    setOperationAction(ISD::FNEG, MVT::v4f32, Expand);
> +    setOperationAction(ISD::FABS, MVT::v4f32, Expand);
>      setOperationAction(ISD::FSQRT, MVT::v4f32, Expand);
>      setOperationAction(ISD::FSIN, MVT::v4f32, Expand);
>      setOperationAction(ISD::FCOS, MVT::v4f32, Expand);
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits

-- 
~Craig

-- 
~Craig

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20121115/ff7c692b/attachment.html>