rjmccall added inline comments.
================
Comment at: clang/lib/CodeGen/CGExprComplex.cpp:964
TestAndClearIgnoreImag();
BinOpInfo Ops;
----------------
rjmccall wrote:
> Please make a helper function to emit an operand as a possibly-promoted
> complex value.
As requested, please introduce this helper function so that here you can just
write:
```
Ops.LHS = EmitPromotedComplexOperand(E->getLHS(), PromotionTy);
Ops.RHS = EmitPromotedComplexOperand(E->getRHS(), PromotionTy);
```
`EmitPromotedComplexOperand` will do what's being done here for each side:
```
if (!E->getType()->isAnyComplexType()) {
return CGF.EmitPromotedScalarExpr(E, PromotionType);
} else {
return CGF.EmitPromotedComplexExpr(E, PromotionType);
}
```
This would rely on making those functions check for a null PromotionType and
just fall back to the normal emission path. Doing that seems it would simplify
a lot of code.
================
Comment at: clang/lib/CodeGen/CGExprComplex.cpp:295
+ CGF.ConvertType(
\
+ E->getType()->castAs<ComplexType>()->getElementType()),
\
+ "unpromotion");
\
----------------
If you pull
`CGF.ConvertType(E->getType()->castAs<ComplexType>()->getElementType())` out
into a helper function, and call it once for both lanes, this will get so much
more concise.
================
Comment at: clang/lib/CodeGen/CGExprComplex.cpp:932
+ return ComplexPairTy(Resultr, Resulti);
+ }
+ // fallback path
----------------
This whole `else` block is identical to the fallback path beneath it.
================
Comment at: clang/lib/CodeGen/CGExprComplex.cpp:940
+ Result.second,
+ CGF.ConvertType(E->getType()->castAs<ComplexType>()->getElementType()));
+ return ComplexPairTy(Resultr, Resulti);
----------------
Like above, please convert the type once for both lanes. Also, I think you
might need to handle null values, because emitters can generate null results if
the result is unused.
================
Comment at: clang/lib/CodeGen/CGExprScalar.cpp:827
+ Value *VisitBin##OP##Assign(const CompoundAssignOperator *E) {
\
+ QualType promotionTy = getPromotionType(E);
\
+ auto result =
\
----------------
Unfortunately, this won't work because the type of the assignment expression is
not necessarily the type of the compound operation. Also, in C the result of
an assignment is the r-value that was actually stored, and in C++ it's an
l-value referring to the LHS; in either case, promoted emission can never
propagate through the expression. So you should just handle promotion
internally to EmitCompoundAssign, rather than passing in a promotion type, and
it will always be expected to return an unpromoted result.
================
Comment at: clang/lib/CodeGen/CGExprScalar.cpp:3117
+ Result.Ty = E->getType();
+ }
Result.Opcode = E->getOpcode();
----------------
This is one of those places that would get simplified if we just made
`EmitPromotedScalarExpr` handle the case of a null promotion type by falling
back to the normal path. All these functions that are used from both the
promoted and normal paths find themselves doing this.
================
Comment at: clang/lib/CodeGen/CGExprScalar.cpp:3132
return CGF.EmitScalarCompoundAssignWithComplex(E, Result);
// Emit the RHS first. __block variables need to have the rhs evaluated
----------------
So, I think what you want to do here is basically compute `getPromotedType` of
all the individual pieces that go into this: the computation result type, the
computation LHS type, and the RHS type.
- `OpInfo.Ty` will be the promoted computation result type, if it exists, or
else the normal one.
- Call `EmitPromoted` on the RHS using the promoted RHS type.
- Convert the loaded LHS to the promoted computation LHS type, if it exists, or
else the normal one.
Then you do the operation and convert from `OpInfo.Ty` back to the LHS type
before you do the assignment.
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
