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

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to