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

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

I think you should also at least support promoted arithmetic through the `_Real` and `_Imag` scalar operators before calling this complete.  That should be simple — just a matter of calling EmitPromotedComplexExpr from the scalar path.



================
Comment at: clang/lib/CodeGen/CGExprComplex.cpp:1005
+  QualType PromotionTypeRHS = getPromotionType(E->getRHS());
+  QualType PromotionTypeLHS =getPromotionType(E->getLHS());
+  QualType PromotionTypeCR;
----------------
whitespace


================
Comment at: clang/lib/CodeGen/CGExprScalar.cpp:3130
+      PromotionTypeCR = CGF.getContext().FloatTy;
+  }
+  QualType PromotionTypeLHS = getPromotionType(E->getLHS());
----------------
Please don't duplicate this computation of the promotion type.  This is just `getPromotionType` except you want something guaranteed non-null so you fall back to the normal computation result type.

You'll need to make `getPromotionType` take a type instead of an expression, but that's a good idea anyway.


================
Comment at: clang/lib/CodeGen/CGExprScalar.cpp:3131
+  }
+  QualType PromotionTypeLHS = getPromotionType(E->getLHS());
+  QualType PromotionTypeRHS = getPromotionType(E->getRHS());
----------------
The difference between `getComputationLHSType()` and `E->getLHS()->getType())` is important here.  This needs to use `getComputationLHSType()`.

In a compound assignment, the value loaded from the LHS has to be promoted to the appropriate type for the computation.  The type it should be promoted to is the computation LHS type.  For promotion, this matters when you have something like `myInt *= myFloat16`; the LHS type will be `int`, but the  computation LHS type will be `_Float16`.  In this mode, you need to be promoting the LHS to `float` before evaluating the `*` operator.

You also have this wrong in the complex emitter.  These things get even more subtle in the complex emitter because IIRC in the scalar cases the computation LHS type is always equal to the computation result type, but in the complex cases it can still be scalar, if you do something like `myFloat *= myComplex`.


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

https://reviews.llvm.org/D113107