bruno added inline comments.
================
Comment at: lib/CodeGen/CGExprScalar.cpp:1042
+ }
+
+ assert(SrcElementTy->isFloatingPointTy() &&
----------------
What happens if the SrcElementTy is float and DstElementTy isn't? Seems like it
will hit the assertion below.
================
Comment at: lib/Sema/SemaExpr.cpp:7511
+ // If this is a compound assignment, allow converting the RHS to the type
+ // of the LHS.
+ if (IsCompAssign && isVector(LHSType, Context.HalfTy)) {
----------------
Since it seems that you're always doing the same conversion (the only variable
input here is the number of elements), can you update the comment to mention
the exact conversion?
================
Comment at: lib/Sema/SemaExpr.cpp:8072
+/// Convert E, which is a vector, to a vector that has a different element
+/// type.
+static ExprResult convertVector(Expr *E, QualType ElementType, Sema &S) {
----------------
What about `Convert vector E to a vector with the same number of elements but
different element type`?
================
Comment at: lib/Sema/SemaExpr.cpp:11316
+// This helper function promotes a binary operator's operands (which are of a
+// half vector type) to a vector of floats and then truncates the result to
+// a vector of either half or short.
----------------
`which are of a half vector type` -> should be there an assertion below to make
sure?
================
Comment at: lib/Sema/SemaExpr.cpp:11329
+ QualType BinOpResTy = RHS.get()->getType();
+ if (isVector(ResultTy, Context.ShortTy))
+ BinOpResTy = S.GetSignedVectorType(BinOpResTy);
----------------
Can you add a comment explaining that a) this conversion from float -> int and
b) it's needed in case the original binop is a comparison?
================
Comment at: lib/Sema/SemaExpr.cpp:11537
+ // Some of the binary operations require promoting operands of half vector
+ // and truncating the result. For now, we do this only when HalfArgsAndReturn
+ // is set (that is, when the target is arm or arm64).
----------------
What about `of half vector and truncating the result` to `of half vector to
float and truncating the result back to half`
================
Comment at: lib/Sema/SemaExpr.cpp:11568
- if (CompResultTy.isNull())
+ if (CompResultTy.isNull()) {
+ if (ConvertHalfVec)
----------------
Please add a comment here explaining that this path only happens when it's a
compound assignment.
================
Comment at: lib/Sema/SemaExpr.cpp:11978
+ // truncating the result. For now, we do this only when HalfArgsAndReturns
+ // is set (that is, when the target is arm or arm64).
+ ConvertHalfVec =
----------------
This same logic is used elsewhere in the patch, perhaps factor it out into a
static function?
https://reviews.llvm.org/D32520
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
