[PATCH] D108832: [Builtins] Support ext_vector_type args for __builtin_fminf.

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

I think I would want some level of examination/analysis as to whether we want to do this with all of the vector-types, instead of just the `ext_vector_type`.



================
Comment at: clang/lib/Sema/SemaChecking.cpp:16544
+                                 E, AA_Passing))
+      return ExprResult(true);
+
----------------
Doing this as ExprResult(true) is absurdly jarring.


================
Comment at: clang/lib/Sema/SemaChecking.cpp:16554
+
+  if (auto *VecType = ElementType->getAs<ExtVectorType>())
+    ElementType = VecType->getElementType();
----------------
I wonder if this should just deal in `VectorType`, which is the type that intends to emulate the GCC vector types.  My understanding is `ExtVectorType` is for the clang-extended vector types.


================
Comment at: clang/lib/Sema/SemaChecking.cpp:16575
+
+  // Update call argument to use the possibly converted matrix argument.
+  TheCall->setArg(0, A.get());
----------------
What does this comment come from?  Is this supposed to be a 'FIXME'?  I don't see any other matrix stuff here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108832