[PATCH] D96904: [IR] restrict vector reduction intrinsic types

[Sanjay Patel via Phabricator via llvm-commits]

spatel created this revision.
spatel added reviewers: craig.topper, aemerson, dmgreen, RKSimon, bero.
Herald added a subscriber: mcrosier.
spatel requested review of this revision.
Herald added a subscriber: jdoerfert.
Herald added a project: LLVM.

I'm not sure if there's a better/preferred way to spell the dual constraints we want in the tablegen defs? The arguments in all cases should be vectors of exactly one of integer or FP.

All of the tests currently pass the verifier because we check for any vector type regardless of the type of reduction. 
This obviously can't work if we mix up integer and FP, and based on current LangRef text it was not intended to work for pointers either.

The pointer case from https://llvm.org/PR49215 is what led me here. That example is still going to crash, but now it crashes sooner in the LoopVectorizer, so we will need another fix there to disallow forming reductions on pointers.


https://reviews.llvm.org/D96904

Files:
  llvm/include/llvm/IR/Intrinsics.td
  llvm/test/Verifier/reduction-intrinsics.ll


Index: llvm/test/Verifier/reduction-intrinsics.ll
===================================================================
--- /dev/null
+++ llvm/test/Verifier/reduction-intrinsics.ll
@@ -0,0 +1,30 @@
+; RUN: not opt -S -verify < %s 2>&1 | FileCheck %s
+
+define float @not_float_reduce(<4 x float> %x) {
+; CHECK: Intrinsic has incorrect argument type!
+  %r = call float @llvm.vector.reduce.umin.v4f32(<4 x float> %x)
+  ret float %r
+}
+
+define i32* @not_pointer_reduce(<4 x i32*> %x) {
+; CHECK: Intrinsic has incorrect argument type!
+  %r = call i32* @llvm.vector.reduce.or.v4p0i32(<4 x i32*> %x)
+  ret i32* %r
+}
+
+define i32 @not_integer_reduce(<4 x i32> %x) {
+; CHECK: Intrinsic has incorrect argument type!
+  %r = call i32 @llvm.vector.reduce.fadd.v4i32(i32 0, <4 x i32> %x)
+  ret i32 %r
+}
+
+define i32* @not_pointer_reduce2(<4 x i32*> %x) {
+; CHECK: Intrinsic has incorrect argument type!
+  %r = call i32* @llvm.vector.reduce.fmin.v4p0i32(<4 x i32*> %x)
+  ret i32* %r
+}
+
+declare float @llvm.vector.reduce.umin.v4f32(<4 x float>)
+declare i32* @llvm.vector.reduce.or.v4p0i32(<4 x i32*>)
+declare i32 @llvm.vector.reduce.fadd.v4i32(i32, <4 x i32>)
+declare i32* @llvm.vector.reduce.fmin.v4p0i32(<4 x i32*>)
Index: llvm/include/llvm/IR/Intrinsics.td
===================================================================
--- llvm/include/llvm/IR/Intrinsics.td
+++ llvm/include/llvm/IR/Intrinsics.td
@@ -1518,32 +1518,32 @@
 
   def int_vector_reduce_fadd : DefaultAttrsIntrinsic<[LLVMVectorElementType<0>],
                                          [LLVMVectorElementType<0>,
-                                          llvm_anyvector_ty]>;
+                                          llvm_anyfloat_ty]>;
   def int_vector_reduce_fmul : DefaultAttrsIntrinsic<[LLVMVectorElementType<0>],
                                          [LLVMVectorElementType<0>,
-                                          llvm_anyvector_ty]>;
+                                          llvm_anyfloat_ty]>;
   def int_vector_reduce_add : DefaultAttrsIntrinsic<[LLVMVectorElementType<0>],
-                                        [llvm_anyvector_ty]>;
+                                        [llvm_anyint_ty]>;
   def int_vector_reduce_mul : DefaultAttrsIntrinsic<[LLVMVectorElementType<0>],
-                                        [llvm_anyvector_ty]>;
+                                        [llvm_anyint_ty]>;
   def int_vector_reduce_and : DefaultAttrsIntrinsic<[LLVMVectorElementType<0>],
-                                        [llvm_anyvector_ty]>;
+                                        [llvm_anyint_ty]>;
   def int_vector_reduce_or : DefaultAttrsIntrinsic<[LLVMVectorElementType<0>],
-                                       [llvm_anyvector_ty]>;
+                                       [llvm_anyint_ty]>;
   def int_vector_reduce_xor : DefaultAttrsIntrinsic<[LLVMVectorElementType<0>],
-                                        [llvm_anyvector_ty]>;
+                                        [llvm_anyint_ty]>;
   def int_vector_reduce_smax : DefaultAttrsIntrinsic<[LLVMVectorElementType<0>],
-                                         [llvm_anyvector_ty]>;
+                                         [llvm_anyint_ty]>;
   def int_vector_reduce_smin : DefaultAttrsIntrinsic<[LLVMVectorElementType<0>],
-                                         [llvm_anyvector_ty]>;
+                                         [llvm_anyint_ty]>;
   def int_vector_reduce_umax : DefaultAttrsIntrinsic<[LLVMVectorElementType<0>],
-                                         [llvm_anyvector_ty]>;
+                                         [llvm_anyint_ty]>;
   def int_vector_reduce_umin : DefaultAttrsIntrinsic<[LLVMVectorElementType<0>],
-                                         [llvm_anyvector_ty]>;
+                                         [llvm_anyint_ty]>;
   def int_vector_reduce_fmax : DefaultAttrsIntrinsic<[LLVMVectorElementType<0>],
-                                         [llvm_anyvector_ty]>;
+                                         [llvm_anyfloat_ty]>;
   def int_vector_reduce_fmin : DefaultAttrsIntrinsic<[LLVMVectorElementType<0>],
-                                         [llvm_anyvector_ty]>;
+                                         [llvm_anyfloat_ty]>;
 }
 
 //===----- Matrix intrinsics ---------------------------------------------===//


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D96904.324427.patch
Type: text/x-patch
Size: 4344 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20210217/7cb3f160/attachment.bin>