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

[Zahira Ammarguellat via Phabricator via cfe-commits]

zahiraam added a comment.

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

> Okay, well, first off, we definitely shouldn't be repeating conditions like `isX86()` all over the place.  What you want to do is to have a general predicate which answers whether we should emit this operation with excess precision; imagine an architecture that wanted to emit `float` operations with `double` and try to design something that would work for both your case and that one.
>
> It looks like you ought to be able to use the abstraction around `BinOpInfo` to good effect here.  You can add a promotion type on the `ScalarExprEmitter` which, if set, tells the emitter that it should actually produce a value of that type.  `EmitBinOps` can recognize that a promotion type is already set and use that type as the operation type instead of the formal type of the expression.  If a promotion type isn't set on the emitter, `EmitBinOps` can try to recognize that it's in the case where it needs a promotion type; it would then set a promotion type before evaluating the operands, but then truncate the result (and clear the promotion type) before returning.  The logic around compound assignment will take some care (and good testing), but it should work the same basic way, and similarly in the complex emitter.

@rjmccall  Thanks for the review. I have updated the patch. There are still a few things  that I am not sure about, so I haven't' changed the complex emitter yet!
I have added as you suggested a promotion type to the ScalarExprEmitter and pushed most of things in EmitBinOps. The one thing that I am not sure about is the truncation. I don't seem able to do that in EmitBinOps since I don't really have a lvalue available. Please let me know if this is in the direction that you expected it to.
Once again thanks for the review and at your convenience please take a look at these latest changes.


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

https://reviews.llvm.org/D113107