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

[Zahira Ammarguellat via Phabricator via cfe-commits]

zahiraam added a comment.

In D113107#3554538 <https://reviews.llvm.org/D113107#3554538>, @rjmccall wrote:

> Hmm.  I'm sorry, seeing the impact of this, I think it's not great to have this `PromotionTy` field; it's too hard to get the invariants right. Let's do this instead:
>
> 1. Add an `EmitPromoted` method to `ScalarExprEmitter`.  The general rule for this method is that it will generate a value of a promoted type, which should be passed in.
> 2. Make `EmitBinOps` take an optional promotion type. If it's provided, it should call `EmitPromoted` instead of `Visit` to emit the operands, and it should set up the promotion type as the formal type of the operation.
> 3. In `EmitPromoted`, call `IgnoreParens` and then split into various cases:
>   - all the promotable floating-point arithmetic expressions (`BinAdd`, `BinSub`, `BinMul`, `BinDiv`, and `UnaryMinus`), which should emit the operands as promoted and then perform a promoted operation, like this:
>
>   return Emit##OP(EmitBinOps(E, promotionTy));
>
>   - anything that promotion should propagate through, which under the standard should not include assignments or casts, but could include things like the comma operator and the ternary operator;
>   - everything else, which should emit the expression normally and then promote the result.
> 1. Change the standard path for the promotable arithmetic expressions (the macro-expanded `VisitBinAdd` etc.) to detect the need to do a promoted operation and then truncate, like this:
>
>   QualType promotionTy = getPromotionType(E);
>   auto result = Emit##OP(EmitBinOps(E, promotionTy));
>   if (result && promotionTy) result = Builder.CreateFPTrunc(result, ConvertType(E->getType()), "unpromotion");
>   return result;

@rjmccall Thanks for that.
I think I am ok for 1. 2. and 4.
Still trying to figure out 3., EmitPromoted. When I make the change 4. the call  to  Emit##OP(EmitBinOps(E, promotionTy)); in VisitBinOP. Is this function still needed? Am i missing something?

Also these changes seem to be only for ScalarExprEmitter not for ComplexExpEmitter. I still need a call to  getPromotionType(E) for ComplexPairTy VisitBinAdd/Sub/Mul/Div , since there are also calling EmitBinOps, right?


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

https://reviews.llvm.org/D113107