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

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.


================
Comment at: clang/lib/CodeGen/CGExprComplex.cpp:609
+  ComplexPairTy Op;
+  PromotionType = getPromotionType(E->getSubExpr()->getType());
+  if (!PromotionType.isNull())
----------------
zahiraam wrote:
> rjmccall wrote:
> > This is overwriting the argument, so the code below doesn't understand whether it's supposed to be emitting a promoted result or not.  You can test this with something like:
> > 
> > ```
> > cf16 = -cf16;
> > ```
> > 
> > Here the context does not want a promoted result, but you will produce one anyway.
> > 
> > Because this operator does its own arithmetic, like the binary operators, it needs to follow the same basic logic:
> > - remember whether you're supposed to emit a promoted result
> > - if not, check whether you should do the arithmetic promoted anyway
> > - if you did the arithmetic promoted, but you're not supposed to emit a promoted result, unpromote the result
> 
> So something like this:
> 
> #define HANDLEUNOP(OP)                                                         \
>   Value *VisitUnary##OP(const UnaryOperator *E,                                \
>                         QualType PromotionType = QualType()) {		             \
>     QualType promotionTy = getPromotionType(E->getSubExpr()->getType());       \
>     llvvm::Value *result = **Visit##OP**(E, promotionTy);                                     \
>    result = EmitUnpromotion(promotionTy, E, result);
>   }
>   HANDLEUNOP(Minus)
>  HANDLEUNOP(Plus)
>  HANDLEUNOP(Imag)
>  HANDLEUNOP(Reg)
> #undef HANDLEUNOP
> 
> Where Visit##* are the bodies taken from the current VisitUnary*?
You should only unpromote if `PromotionType` was null, but yes, that's the idea.  I don't think you should macro it, though.


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

https://reviews.llvm.org/D113107