[PATCH] D111985: [Clang] Add elementwise min/max builtins.

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.


================
Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:8747-8748
 
+def err_elementwise_math_arg_types_mismatch : Error <
+  "argument types do not match, %0 != %1">;
+
----------------
Does `err_typecheck_call_different_arg_types` suffice?


================
Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:8750-8751
+
+def err_elementwise_math_invalid_arg_type: Error <
+  "argument type %0 is not supported">;
+
----------------
It'd be nice to avoid this entirely as the diagnostic situation is one we would have elsewhere (so we should be able to use common diagnostic checking code from here and places like `SemaBuiltinMatrixColumnMajorLoad()` ideally).


================
Comment at: clang/lib/Sema/SemaChecking.cpp:16662-16667
+  if (!Ty->getAs<VectorType>() && !ConstantMatrixType::isValidElementType(Ty)) {
+    S.Diag(Loc, diag::err_elementwise_math_invalid_arg_type) << Ty;
+    return true;
+  }
+  return false;
+}
----------------
Related to the comment above on the diagnostic, I'm wondering if we want to abstract this into a helper function that gets used by all the elementwise builtins you're adding?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D111985/new/

https://reviews.llvm.org/D111985