[cfe-commits] RFC: Half floating point support

[Eli Friedman]

On Tue, Oct 4, 2011 at 2:08 PM, Anton Korobeynikov
<anton at korobeynikov.info> wrote:
> Hello Everyone
>
> Attached is a preliminary patch for storage-only half floating point
> support in clang. Basically, lack of such support is a regression now,
> because llvm-gcc supported it.
>
> The key idea of the patch is the following: when building AST for new
> 'half' type implicit conversion to float is performed (the opposite
> conversion thus will be done automatically). Such a conversion,
> fortunately, should be done only in few places:
>
> 1. During lvalue-to-rvalue conversion
> 2. During building initializer sequence
> 3. For some unary ops where no lvalue-to-rvalue conversion is
> performed (e.g. ++foo).
>
> Later these promotions can be turned off depending on whether the
> target wants half fp to be native or storage only type.
>
> During the codegen I just turn such float<->half conversion into
> appropriate intrinsic calls plus some other minor stuff for
> initializers.
>
> What is missed in the patch:
>
> 1. Sema checks.
> 2. Testsuite (I have extensive one, but I don't have for sema checks)
> 3. (Maybe) Some C++ bits.... Most probably RTTI?
>
> So, I just wanted thoughts and comments on this patch.
>
> Answering the very first question: why this is not done entirely in
> codegen phase? Answer is pretty simple: there are a lot of EmitFoo()
> methods, so I'd need to modify all of them and this looked pretty
> fragile and inconvenient. Also, I thought it's pretty important to
> have everything in AST (implicit conversions to float) instead of
> codegen phase magic.
>
> Thanks!

This looks like it's generally in the right direction.


@@ -8170,6 +8177,12 @@ ExprResult
Sema::CreateBuiltinUnaryOp(SourceLocation OpLoc,
                                                 Opc == UO_PostInc,
                                                 Opc == UO_PreInc ||
                                                 Opc == UO_PreDec);
+    // Half FP is a bit different: it's a storage-only type, meaning that any
+    // "use" of it should involve promotion to float.
+    if (resultType->isHalfType()) {
+      Input = ImpCastExprToType(Input.take(), Context.FloatTy,
CK_FloatingCast);
+      resultType = Context.FloatTy;
+    }
     break;
   case UO_AddrOf:
     resultType = CheckAddressOfOperand(*this, Input.get(), OpLoc);

This looks suspicious: specifically, it destroys the invariant in C++
that preinc/dec return an lvalue.  Also, it doesn't appear to handle
++x and x+=1 equivalently.

Besides that, I don't see anything obviously wrong; did you make sure
you aren't introducing any -Wswitch-enum warnings?

-Eli