[PATCH] D113107: Support of expression granularity for _Float16.

John McCall via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Nov 15 18:07:31 PST 2021


rjmccall added a comment.

Can we split the patch which changes what types are enabled for various x86 sub-targets out from the patch that changes the semantics of operations on `_Float16`?  Or is there a good reason it's disabled currently, namely because the semantics are wrong?



================
Comment at: clang/lib/CodeGen/CGExprScalar.cpp:1315
+  if ((SrcType->isHalfType() || iSFloat16Allowed) &&
+      !CGF.getContext().getLangOpts().NativeHalfType) {
     // Cast to FP using the intrinsic if the half type itself isn't supported.
----------------
Okay, this condition is pretty ridiculous to be repeating in three different places across the compiler.  Especially since you're going to change it when you implement the new option, right?

Can we state this condition more generally?  I'm not sure why this is so narrowly restricted, and the variable name isn't telling me anything, since `_Float16` must by definition be "allowed" if we have an expression of `_Float16` type.


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

https://reviews.llvm.org/D113107



More information about the cfe-commits mailing list