[PATCH] D126377: [clang][AArch64][SVE] Improve diagnostics for SVE operators

[David Truby via Phabricator via cfe-commits]

DavidTruby added inline comments.


================
Comment at: clang/lib/Sema/SemaExpr.cpp:10609-10614
-  if (RHSType->isVLSTBuiltinType() && !LHSType->isVLSTBuiltinType()) {
-    auto DestType = tryScalableVectorConvert((IsCompAssign ? nullptr : &LHS),
-                                             LHSType, RHSType);
-    if (DestType == QualType())
-      return InvalidOperands(Loc, LHS, RHS);
-    return DestType;
----------------
c-rhodes wrote:
> why is this removed?
This is sort of an artefact of splitting this and D126380; essentially that patch was motivated by the fact that this code here doesn't really do anything meaningful for scalable vectors. I added it for symmetry with non-scalable vectors but the triggers for conversions here don't really fire in meaningful code.
Doing this properly is resolved in D126380 so I could remove it there instead but this code didn't really work and doesn't have tests anyway.


================
Comment at: clang/test/Sema/aarch64-sve-vector-arith-ops.c:23
-  (void)(i8 + f64); // expected-error{{invalid operands to binary expression}}
-  (void)(i8 + 0);   // expected-error{{invalid operands to binary expression}}
-  (void)(i8 + 0l);  // expected-error{{invalid operands to binary expression}}
----------------
c-rhodes wrote:
> I think these vector + imm tests should be removed in D126380 but fine to keep here if it's easier
Ah yes I missed this, you're correct but I will be pushing the two patches at the same time so I guess it doesn't matter much except for correct history? Happy to change it though.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D126377/new/

https://reviews.llvm.org/D126377