[PATCH] D10964: [Codegen] Add intrinsics 'hadd*' and corresponding SDNodes for horizontal sum operation.

James Molloy james.molloy at arm.com
Tue Jul 7 02:45:22 PDT 2015


jmolloy requested changes to this revision.

This revision now requires changes to proceed.

Hi,

Generally looks good, but I have some initial comments.

Cheers,

James


================
Comment at: docs/LangRef.rst:9593
@@ +9592,3 @@
+      declare <4 x integer> @llvm.hadd.v4i32(<4 x integer> %a)
+      declare <4 x float> @llvm.hadd.v4f32(<4 x float> %a)
+
----------------
You need to be very explicit about the behaviour of this intrinsic with floating point arguments. What order, if any, does it perform the adds in? If there is no guaranteed order, it can only be used in fast-math mode.

================
Comment at: include/llvm/IR/Intrinsics.td:599
@@ +598,3 @@
+// Calculate the horizontal/reduction sum across the elements of input vector.
+def int_hadd_int : Intrinsic<[llvm_anyint_ty], [llvm_anyvector_ty], [IntrNoMem]>;
+def int_hadd_float : Intrinsic<[llvm_anyfloat_ty], [llvm_anyvector_ty], [IntrNoMem]>;
----------------
Just having one intrinsic here would be good; there's no need for a separate int and float version.

================
Comment at: include/llvm/IR/Intrinsics.td:601
@@ -598,2 +600,3 @@
+def int_hadd_float : Intrinsic<[llvm_anyfloat_ty], [llvm_anyvector_ty], [IntrNoMem]>;
 //===-------------------------- Masked Intrinsics -------------------------===//
 //
----------------
Blank line missing here.

================
Comment at: lib/CodeGen/SelectionDAG/LegalizeVectorOps.cpp:712
@@ -707,1 +711,3 @@
+  case ISD::FHADD:
+    return UnrollHADD(Op);
   default:
----------------
Can't you just call ExpandHADD() here? or at least share the unroll and expand code?


Repository:
  rL LLVM

http://reviews.llvm.org/D10964







More information about the llvm-commits mailing list