This is an archive of the discontinued LLVM Phabricator instance.

[clang][AArch64][SVE] Improve diagnostics for SVE operators
ClosedPublic

Authored by DavidTruby on May 25 2022, 6:50 AM.

Details

Summary

This patch corrects some diagnostics for the SVE sizeless vector
operators, including correctly diagnosing when the vectors are
different sizes.

Diff Detail

Event Timeline

DavidTruby created this revision.May 25 2022, 6:50 AM
DavidTruby requested review of this revision.May 25 2022, 6:50 AM
Herald added a project: Restricted Project. · View Herald TranscriptMay 25 2022, 6:50 AM
Herald added a subscriber: cfe-commits. · View Herald Transcript
Matt added a subscriber: Matt.May 25 2022, 6:51 AM
c-rhodes added inline comments.May 26 2022, 12:53 AM
clang/lib/Sema/SemaExpr.cpp
10609–10614

why is this removed?

10614

missing a test for this?

clang/test/Sema/aarch64-sve-vector-arith-ops.c
23

I think these vector + imm tests should be removed in D126380 but fine to keep here if it's easier

DavidTruby added inline comments.May 26 2022, 6:21 AM
clang/lib/Sema/SemaExpr.cpp
10609–10614

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.

10614

I wrote this test but seem to have forgotten to add it... Will do that!

clang/test/Sema/aarch64-sve-vector-arith-ops.c
23

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.

c-rhodes added inline comments.May 26 2022, 6:58 AM
clang/test/Sema/aarch64-sve-vector-arith-ops.c
23

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.

Yeah no worries keep them in here, thanks for clarifying

DavidTruby updated this revision to Diff 434431.Jun 6 2022, 4:48 AM

add tests for vector non-scalar diagnostic

c-rhodes accepted this revision.Jun 6 2022, 5:06 AM

LGTM

This revision is now accepted and ready to land.Jun 6 2022, 5:06 AM
This revision was landed with ongoing or failed builds.Jun 7 2022, 7:35 AM
This revision was automatically updated to reflect the committed changes.