rjmccall added a comment.
Somehow we've taken a huge step back on unpromotion, and I'm worried you're now
doing the exact thing I didn't want us doing and forcing all the downstream
clients to handle the possibility of a promoted result.
================
Comment at: clang/lib/CodeGen/CGExprComplex.cpp:418
+ EmitStore(Val, lvalue.getAddress(CGF), lvalue.getType(),
+ lvalue.isVolatileQualified());
}
----------------
You should not be changing this code; clients should be expected to give you a
value that matches the type of the l-value, which generally means they need to
be unpromoting.
================
Comment at: clang/lib/CodeGen/CGExprComplex.cpp:615
+ ComplexPairTy result = VisitMinus(E, promotionTy);
+ return result;
+}
----------------
This is not unpromoting if the original `PromotionType` is null.
================
Comment at: clang/lib/CodeGen/CGExprComplex.cpp:613
+ result = EmitUnpromotion(promotionTy, E->getSubExpr()->getType(), result);
+ return result;
+}
----------------
rjmccall wrote:
> zahiraam wrote:
> > rjmccall wrote:
> > > You should unpromote only if we're not in a promoted context, which is to
> > > say, only if the `PromotionType` that was passed in is null.
> > oh! right. The promotionTy is not even used in the function. Thanks.
> I'm sorry, but this is not the change I requested; I meant you need to do
> this:
>
> ```
> QualType promotionTy = PromotionType.isNull() ?
> getPromotionType(E->getType()) : PromotionType;
> ComplexPairTy result = VisitMinus(E, promotionTy);
> if (PromotionType.isNull())
> result = EmitUnpromotion(promotionTy, result);
> return result;
> ```
>
> The fact that you weren't seeing problems because of this makes me concerned.
This is not unpromoting if the original `PromotionType` is null.
================
Comment at: clang/lib/CodeGen/CGExprComplex.cpp:296
+ ComplexPairTy EmitUnpromotion(QualType promotionTy, const BinaryOperator *E,
+ ComplexPairTy &result) {
+ if (!promotionTy.isNull()) {
----------------
rjmccall wrote:
> Please take `result` by value instead of by reference; it's surprising that
> this both returns the value and modifies the parameter.
Where is unpromotion happening now?
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
