[llvm-commits] [PATCH] ARM: Custom lower v2f32 = fp_round to fix crash with NEON

[David Peixotto]

LGTM. Thanks again for the patch.

 

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

 

 

From: Pete Couperus [mailto:pjcoup at gmail.com] 
Sent: Wednesday, November 14, 2012 9:39 PM
To: David Peixotto
Cc: llvm-commits at cs.uiuc.edu
Subject: Re: [llvm-commits] [PATCH] ARM: Custom lower v2f32 = fp_round to
fix crash with NEON

 

Hi David,

Thanks again for taking a look.  I've modified the patch to address your
points.  You're right, the docs indicate that Fixed Size Arrays are
preferred over SmallVectors in this case.  Seems that both have been used in
similar cases.
Thanks!

Pete
  

On Wed, Nov 14, 2012 at 5:47 PM, David Peixotto <dpeixott at codeaurora.org>
wrote:

Hi Pete,

 

Thanks for fixing this bug! I have a few minor comments on the patch below.

Index: lib/Target/ARM/ARMISelLowering.cpp

===================================================================

--- lib/Target/ARM/ARMISelLowering.cpp            (revision 167913)

+++ lib/Target/ARM/ARMISelLowering.cpp         (working copy)

+SDValue ARMTargetLowering::LowerFP_ROUND(SDValue Op, SelectionDAG &DAG)
const {

+  assert(Op.getValueType() == MVT::v2f32 &&

+         "FP_ROUND custom lowering should only occur for v2f32.");

+

+  DebugLoc DL = Op.getDebugLoc();

+  SmallVector<SDValue, 2> Vals;

 

I think a fixed size array would work fine here. It is preferred over
SmallVector in the llvm programmers manual:
http://llvm.org/docs/ProgrammersManual.html#ds_sequential

 

+  for(unsigned i = 0; i < 2; i++) {

+    SDValue Val = DAG.getNode(ISD::EXTRACT_VECTOR_ELT, DL, MVT::f64,

+                              Op.getOperand(0), DAG.getConstant(i,
MVT::i32));

+    Vals.push_back(DAG.getNode(ISD::FP_ROUND, DL, MVT::f32,

+                               Val, DAG.getIntPtrConstant(0)));

+  }

+  EVT VecVT = EVT::getVectorVT(*DAG.getContext(), MVT::f32, 2);

+  return DAG.getNode(ISD::BUILD_VECTOR, DL, VecVT, &Vals[0], 2);

+}

+

 

Index: test/CodeGen/ARM/neon_fpconv.ll

===================================================================

--- test/CodeGen/ARM/neon_fpconv.ll                (revision 0)

+++ test/CodeGen/ARM/neon_fpconv.ll             (revision 0)

@@ -0,0 +1,9 @@

+; RUN: llc < %s -march=arm -mattr=+neon | FileCheck %s

 

Please add a comment that this test is for PR12540.

 

+

+define <2 x float> @vtrunc(<2 x double> %a) {

+; CHECK: vcvt.f32.f64 [[S0:s[0-9]+]], [[D0:d[0-9]+]]

+; CHECK: vcvt.f32.f64 [[S1:s[0-9]+]], [[D1:d[0-9]+]]

+  %vt = fptrunc <2 x double> %a to <2 x float>

+  ret <2 x float> %vt

+}

+

 

Thanks,

David

 

-- 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 Pete Couperus
Sent: Tuesday, November 13, 2012 11:24 PM
To: llvm-commits at cs.uiuc.edu
Subject: [llvm-commits] [PATCH] ARM: Custom lower v2f32 = fp_round to fix
crash with NEON

 

Hello,

Lowering "fptrunc <2 x double> %a to <2 x float>" is broken on ARM with
NEON.
This patch custom lowers this conversion using two single element vcvt's.
This fixes the following PRs.

http://llvm.org/bugs/show_bug.cgi?id=12540
http://llvm.org/bugs/show_bug.cgi?id=13964

Please review!
Thanks.

Pete

 

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