zahiraam added inline comments.
================
Comment at: clang/lib/CodeGen/CGExprComplex.cpp:950
+ComplexExprEmitter::EmitPromotedComplexOperand(const Expr *E,
+ QualType PromotionType) {
+ if (E->getType()->isAnyComplexType()) {
----------------
rjmccall wrote:
> Maybe call this the `OverallPromotionType` to make it clear that it's not
> necessarily the right promotion type for the operand (as in fact it isn't in
> the scalar case).
Changed it, but not sure it makes it clearer.
================
Comment at: clang/lib/CodeGen/CGExprScalar.cpp:3112
+ result = CGF.Builder.CreateFPExt(result, ConvertType(E->getType()));
+ return result;
+}
----------------
rjmccall wrote:
> The first version of the fallback path (the one in the `else`) is correct.
> Like with the complex emitter, though, you've got a bug where fall through
> from the earlier `switch` ends up in the second path; test case is something
> like `f16 + (true, f16)`.
>
> The best fix is to just not try to use `else` for your fallback path. The
> earlier blocks will return if they recognize cases that they can specially
> handle; everything should fall out, and you handle everything that reaches
> that point with the fallback path.
>
> This code should check for `UnaryOperator` like it does for `BinaryOperator`
> and delegate to your `_Real` / `_Imag` implementations above. You should
> probably also handle unary `+` and `-` so that using those does not cause
> intermediate truncations.
OK. Fixed the fallback part. But I can't find a test case that would make me
land into the switch for UnaryOperator . The code I added is not exercised.
Would you mind suggesting a test case? Not sure about the f16 + (true, f16)
above?
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D113107/new/
https://reviews.llvm.org/D113107
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
