[Clang] Add __builtin_reduce_xor

[junaire]

Summary

This patch implements __builtin_reduce_xor as specified in D111529.

[fhahn]

clang/lib/Sema/SemaChecking.cpp

2112

I think reduce_xor is only specified for integer types, so I think we need an extra check here?

[junaire]

• Add extra type check to the vector elements.
• Add comment about type restriction.
• polish tests a little bit.

[fhahn]

clang/lib/Sema/SemaChecking.cpp

2116

nit: the element type will be restricted to int, so maybe say something like only supports vectors of integers>

clang/test/Sema/builtins-reduction-math.c

50

vector of integers type sounds a bit off. Not sure if that's much better, but how about dropping the type from the end?

[junaire]

Improve diagnostic message.

Fix failing CI by running:
arc diff git merge-base HEAD origin --update D115231

[junaire]

Fix wrong code logic.

[junaire]

Ping.

[RKSimon]

Any reason not to handle builtin_reduce_xor + builtin_reduce_or as well in the same patch since they are very similar?

[junaire]

In D115231#3194372, @RKSimon wrote:

Any reason not to handle builtin_reduce_xor + builtin_reduce_or as well in the same patch since they are very similar?

Hi, thanks for take a look at this.
Yes, you're right. The reason I split them is that I'm a beginner, so I want to send a "experimental patch" first, this will make review work a little bit easier if I mess something up. I think it's reasonable to add all similar builtins if currently nothing wrong about this one.

[aaron.ballman]

clang/lib/Sema/SemaChecking.cpp

2115

Please spell out the type here (same below).

2116

No need to repeat the type name here (same below).

[junaire]

Explicitly speak out types for Arg, but don't repeat the vector type name.

[aaron.ballman]

LGTM aside from a nit. Please wait a day or two before landing in case other reviewers have concerns they haven't had the chance to raise yet.

clang/include/clang/Sema/Sema.h

12773 ↗

(On Diff #394563)

If we're going to be renaming this, we should probably drop the Sema prefix from the name. (I'd note that we have a lot of functions prefixed with Sema that should be renamed, in case anyone really wants an NFC task that should be pretty trivial.)

[junaire]

Rename SemaBuiltinReduceMathPreCheck to PrepareBuiltinReduceMathOneArg

[RKSimon]

In D115231#3194439, @junaire wrote:

In D115231#3194372, @RKSimon wrote:

Any reason not to handle builtin_reduce_xor + builtin_reduce_or as well in the same patch since they are very similar?

Hi, thanks for take a look at this.
Yes, you're right. The reason I split them is that I'm a beginner, so I want to send a "experimental patch" first, this will make review work a little bit easier if I mess something up. I think it's reasonable to add all similar builtins if currently nothing wrong about this one.

That's fine - I'm happy to see these addressed as follow up patches - cheers!

[fhahn]

LGTM, thanks

[junaire]

Thanks for accepting this patch! I would appreciate it if someone is willing to commit this for me since I don't have commit access ;D

You can use:
Jun Zhang
[e-mail]

[fhahn]

In D115231#3197112, @junaire wrote:

Thanks for accepting this patch! I would appreciate it if someone is willing to commit this for me since I don't have commit access ;D

You can use:
Jun Zhang
[e-mail]

It looks like the patch doesn't apply cleanly on current main. Could you rebase the patch?

[junaire]

• rebase to main.
• rename PrepareBuiltinReduceMathOneArg to PrepareBuiltinReduceMathOneArgCall.

clang/include/clang/Basic/Builtins.def

 BUILTIN(__builtin_alloca_with_align, "v*zIz", "Fn")
 BUILTIN(__builtin_call_with_static_chain, "v.", "nt")
 
 BUILTIN(__builtin_elementwise_abs, "v.", "nct")
 BUILTIN(__builtin_elementwise_max, "v.", "nct")
 BUILTIN(__builtin_elementwise_min, "v.", "nct")
 BUILTIN(__builtin_reduce_max, "v.", "nct")
 BUILTIN(__builtin_reduce_min, "v.", "nct")
+BUILTIN(__builtin_reduce_xor, "v.", "nct")
 
 BUILTIN(__builtin_matrix_transpose, "v.", "nFt")
 BUILTIN(__builtin_matrix_column_major_load, "v.", "nFt")
 BUILTIN(__builtin_matrix_column_major_store, "v.", "nFt")
 
 // "Overloaded" Atomic operator builtins.  These are overloaded to support data
 // types of i8, i16, i32, i64, and i128.  The front-end sees calls to the
 // non-suffixed version of these (which has a bogus type) and transforms them to

clang/include/clang/Basic/DiagnosticSemaKinds.td

 def err_builtin_launder_invalid_arg : Error<
   "%select{non-pointer|function pointer|void pointer}0 argument to "
   "'__builtin_launder' is not allowed">;
 
 def err_builtin_invalid_arg_type: Error <
   "%ordinal0 argument must be a "
   "%select{vector, integer or floating point type|matrix|"
    "pointer to a valid matrix element type|"
-   "signed integer or floating point type|vector type}1 (was %2)">;
+   "signed integer or floating point type|vector type|"
+   "vector of integers}1 (was %2)">;
 
 def err_builtin_matrix_disabled: Error<
   "matrix types extension is disabled. Pass -fenable-matrix to enable it">;
 def err_matrix_index_not_integer: Error<
   "matrix %select{row|column}0 index is not an integer">;
 def err_matrix_index_outside_range: Error<
   "matrix %select{row|column}0 index is outside the allowed range [0, %1)">;
 def err_matrix_incomplete_index: Error<

clang/lib/CodeGen/CGBuiltin.cpp

case Builtin::BI__builtin_reduce_min: {
     };
     Value *Op0 = EmitScalarExpr(E->getArg(0));
     Value *Result = Builder.CreateUnaryIntrinsic(
         GetIntrinsicID(E->getArg(0)->getType(), Op0->getType()), Op0, nullptr,
         "rdx.min");
     return RValue::get(Result);
   }
 
+  case Builtin::BI__builtin_reduce_xor: {
+    Value *Op0 = EmitScalarExpr(E->getArg(0));
+    Value *Result = Builder.CreateUnaryIntrinsic(
+        llvm::Intrinsic::vector_reduce_xor, Op0, nullptr, "rdx.xor");
+    return RValue::get(Result);
+  }
+
   case Builtin::BI__builtin_matrix_transpose: {
     const auto *MatrixTy = E->getArg(0)->getType()->getAs<ConstantMatrixType>();
     Value *MatValue = EmitScalarExpr(E->getArg(0));
     MatrixBuilder<CGBuilderTy> MB(Builder);
     Value *Result = MB.CreateMatrixTranspose(MatValue, MatrixTy->getNumRows(),
                                              MatrixTy->getNumColumns());
     return RValue::get(Result);
   }

clang/lib/Sema/SemaChecking.cpp

case Builtin::BI__builtin_elementwise_abs:
     break;
   case Builtin::BI__builtin_elementwise_min:
   case Builtin::BI__builtin_elementwise_max:
     if (SemaBuiltinElementwiseMath(TheCall))
       return ExprError();
     break;
   case Builtin::BI__builtin_reduce_max:
   case Builtin::BI__builtin_reduce_min:
     if (SemaBuiltinReduceMath(TheCall))
       return ExprError();
     break;
+
+  // __builtin_reduce_xor supports vector of integers only.
+  case Builtin::BI__builtin_reduce_xor: {
+    if (SemaBuiltinReduceMath(TheCall))
+      return ExprError();
+
+    const auto *Arg = TheCall->getArg(0);
+    if (!Arg->getType()
+             ->getAs<VectorType>()
+             ->getElementType()
+             ->isIntegerType()) {
+      Diag(Arg->getBeginLoc(), diag::err_builtin_invalid_arg_type)
+          << 1 /* vector of integers */ << 5 << Arg->getType();
+      return ExprError();
+    }
+    break;
+  }
+
   case Builtin::BI__builtin_matrix_transpose:
     return SemaBuiltinMatrixTranspose(TheCall, TheCallResult);
 
   case Builtin::BI__builtin_matrix_column_major_load:
     return SemaBuiltinMatrixColumnMajorLoad(TheCall, TheCallResult);
 
   case Builtin::BI__builtin_matrix_column_major_store:
     return SemaBuiltinMatrixColumnMajorStore(TheCall, TheCallResult);

clang/test/CodeGen/builtins-reduction-math.c

void test_builtin_reduce_min(float4 vf1, si8 vi1, u4 vu1) {
   const double r4 = __builtin_reduce_min(vf1_as_one);
 
   // CHECK:      [[CVI1:%.+]] = load <8 x i16>, <8 x i16>* %cvi1, align 16
   // CHECK-NEXT: [[RDX2:%.+]] = call i16 @llvm.vector.reduce.smin.v8i16(<8 x i16> [[CVI1]])
   // CHECK-NEXT: sext i16 [[RDX2]] to i64
   const si8 cvi1 = vi1;
   unsigned long long r5 = __builtin_reduce_min(cvi1);
 }
+
+void test_builtin_reduce_xor(si8 vi1, u4 vu1) {
+
+  // CHECK:      [[VI1:%.+]] = load <8 x i16>, <8 x i16>* %vi1.addr, align 16
+  // CHECK-NEXT: call i16 @llvm.vector.reduce.xor.v8i16(<8 x i16> [[VI1]])
+  short r2 = __builtin_reduce_xor(vi1);
+
+  // CHECK:      [[VU1:%.+]] = load <4 x i32>, <4 x i32>* %vu1.addr, align 16
+  // CHECK-NEXT: call i32 @llvm.vector.reduce.xor.v4i32(<4 x i32> [[VU1]])
+  unsigned r3 = __builtin_reduce_xor(vu1);
+}

clang/test/Sema/builtins-reduction-math.c

void test_builtin_reduce_min(int i, float4 v, int3 iv) {
   // expected-error@-1 {{too many arguments to function call, expected 1, have 2}}
 
   i = __builtin_reduce_min();
   // expected-error@-1 {{too few arguments to function call, expected 1, have 0}}
 
   i = __builtin_reduce_min(i);
   // expected-error@-1 {{1st argument must be a vector type (was 'int')}}
 }
+
+void test_builtin_reduce_xor(int i, float4 v, int3 iv) {
+  struct Foo s = __builtin_reduce_xor(iv);
+  // expected-error@-1 {{initializing 'struct Foo' with an expression of incompatible type 'int'}}
+
+  i = __builtin_reduce_xor();
+  // expected-error@-1 {{too few arguments to function call, expected 1, have 0}}
+
+  i = __builtin_reduce_xor(iv, iv);
+  // expected-error@-1 {{too many arguments to function call, expected 1, have 2}}
+
+  i = __builtin_reduce_xor(i);
+  // expected-error@-1 {{1st argument must be a vector of integers (was 'int')}}
+
+  i = __builtin_reduce_xor(v);
+  // expected-error@-1 {{1st argument must be a vector of integers (was 'float4' (vector of 4 'float' values))}}
+}