rjmccall added a comment.
Thanks, this is looking very close now. Please handle the unary minus case in
the complex emitter as well; I think that's all we need in terms of basic
features.
================
Comment at: clang/lib/CodeGen/CGExprComplex.cpp:296
+ ComplexPairTy EmitUnpromotion(QualType promotionTy, const BinaryOperator *E,
+ ComplexPairTy &result) {
+ if (!promotionTy.isNull()) {
----------------
Please take `result` by value instead of by reference; it's surprising that
this both returns the value and modifies the parameter.
================
Comment at: clang/lib/CodeGen/CGExprScalar.cpp:811
+ if (ElementType->isFloat16Type())
+ return CGF.getContext().getComplexType(CGF.getContext().FloatTy);
+ }
----------------
This path needs to be gated by `shouldEmitFloat16WithExcessPrecision()`.
================
Comment at: clang/lib/CodeGen/CGExprScalar.cpp:3059
+ return CGF.EmitLoadOfLValue(CGF.EmitLValue(E), E->getExprLoc())
+ .getScalarVal();
+ }
----------------
Please do something like:
```
llvm::Value *result = CGF.EmitComplexExpr(Op, /*IgnoreReal*/IgnoreResultAssign,
/*IgnoreImag*/true).first;
if (result && !PromotionType.isNull())
result = EmitPromotedValue(result);
return result;
```
================
Comment at: clang/lib/CodeGen/CGExprScalar.cpp:3082
+ return CGF.EmitLoadOfLValue(CGF.EmitLValue(E), E->getExprLoc())
+ .getScalarVal();
+ }
----------------
Uh. The existing code here is just wrong; I wonder if it's never been tested.
`_Imag` is not a commonly-used operator, especially on l-values.
Please do something like:
```
llvm::Value *result = CGF.EmitComplexExpr(Op, /*IgnoreReal*/true,
/*IgnoreImag*/IgnoreResultAssign).second;
if (result && !PromotionType.isNull())
result = EmitPromotedValue(result);
return result;
```
================
Comment at: clang/lib/CodeGen/CGExprScalar.cpp:3140
+ return CGF.Builder.CreateFPExt(result, ConvertType(E->getType()));
+ }
+ return result;
----------------
Please extract this block out as:
```
llvm::Value *EmitPromotedValue(llvm::Value *result, QualType PromotionType);
```
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
