aaron.ballman added inline comments.
================
Comment at: clang/lib/AST/ExprConstant.cpp:2699
+ RHSValue.getInt(), Result);
+ assert(LHSValue.getKind() == APValue::Float && "SHhuld be no other options");
+ return handleLogicalOpForVector(LHSValue.getFloat(), Opcode,
----------------
typo: Should
================
Comment at: clang/lib/AST/ExprConstant.cpp:2741
+ RHSValue.getInt(), Result);
+ assert(LHSValue.getKind() == APValue::Float && "SHhuld be no other options");
+ return handleCompareOpForVectorHelper(LHSValue.getFloat(), Opcode,
----------------
Typo: Should
================
Comment at: clang/lib/AST/ExprConstant.cpp:2754
+
+ const VectorType *VT = E->getType()->castAs<VectorType>();
+ unsigned NumElements = VT->getNumElements();
----------------
`const auto *`
================
Comment at: clang/lib/AST/ExprConstant.cpp:2778
+ if (EltTy->isIntegerType()) {
+
+ APSInt EltResult{Info.Ctx.getIntWidth(EltTy),
----------------
Might as well remove the spurious whitespace.
================
Comment at: clang/lib/AST/ExprConstant.cpp:2782
+
+ if (BinaryOperator::isLogicalOp(Opcode)) {
+ if (!handleLogicalOpForVector(LHSElt, Opcode, RHSElt, EltResult)) {
----------------
How about:
```
bool Success = true;
if (BinaryOperator::isLogicalOp(Opcode))
Success = handleLogicalOpForVector(...);
else if (BinaryOperator::isComparisonOp(Opcode))
Success = handleCompareOpForVector(...);
else
Success = handleIntIntBinOp(...);
if (!Success) {
Info.FFDiag(E);
return false;
}
ResultElements.push_back(...);
```
================
Comment at: clang/lib/AST/ExprConstant.cpp:2799
+ }
+ ResultElements.push_back(APValue(EltResult));
+
----------------
`emplace_back()` instead?
================
Comment at: clang/lib/AST/ExprConstant.cpp:2800
+ ResultElements.push_back(APValue(EltResult));
+
+ } else if (EltTy->isFloatingType()) {
----------------
Spurious newline?
================
Comment at: clang/lib/AST/ExprConstant.cpp:2813
+
+ ResultElements.push_back(APValue(LHSFloat));
+ }
----------------
`emplace_back()` instead?
================
Comment at: clang/lib/AST/ExprConstant.cpp:9688
+ bool VisitBinaryOperator(const BinaryOperator *E);
+ // FIXME: Missing: unary -, unary ~,
// conditional operator (for GNU conditional select),
----------------
Can you re-flow the rest of the comment now that it's been changed?
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D79755/new/
https://reviews.llvm.org/D79755
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
