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

[John McCall via Phabricator via llvm-commits]

rjmccall added a comment.

In D113107#3556886 <https://reviews.llvm.org/D113107#3556886>, @zahiraam wrote:

> 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?

`VisitBin##OP`, which is the normal emission path, doesn't get a promotion type propagated into it.  If you have an expression like `(a + 6*b) + c`, you'll be in this path when you visit the outermost `+`.  In that path, you'll recognize that you should do promoted emission, so you'll call `EmitPromoted` on the sub-expressions, and then `EmitPromoted` will keep finding expressions with specialized handling for promotion (and therefore calling `EmitPromoted` recursively on their operands) until you get down to something that doesn't.  The normal path will produce a `half`, but the promoted path will produce a `float`.

> 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?

Yeah, you'll do basically the same thing in `ComplexExprEmitter`.  Oh, and I guess as part of that you'll want to be able to emit the conversion expressions between complex and scalar types (`_Imag` etc.) without truncating back to `half`, which means you'll need to add `EmitPromotedScalarExpr` and `EmitPromotedComplexExpr` methods to `CodeGenFunction`.


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

https://reviews.llvm.org/D113107