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

John McCall via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Jun 29 15:09:26 PDT 2022

rjmccall added inline comments.

Comment at: clang/lib/CodeGen/CGExprComplex.cpp:964
   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.



More information about the cfe-commits mailing list