[PATCH] D46688: [AArch64][SVE] Improve diagnostics for vectors with incorrect element-size.

[Sjoerd Meijer via Phabricator via llvm-commits]

SjoerdMeijer accepted this revision.
SjoerdMeijer added inline comments.
This revision is now accepted and ready to land.


================
Comment at: test/MC/AArch64/SVE/ld1b-diagnostics.s:118
 ld1b z0.d, p0/z, [x0, z0.b]
-// CHECK: [[@LINE-1]]:{{[0-9]+}}: error: invalid shift/extend specified, expected 'z[0..31].d, (uxtw|sxtw)'
+// CHECK: [[@LINE-1]]:{{[0-9]+}}: error: invalid operand
 // CHECK-NEXT: ld1b z0.d, p0/z, [x0, z0.b]
----------------
sdesmalen wrote:
> SjoerdMeijer wrote:
> > Is this a regression? Is the old diagnostic more accurate? If so, can we avoid this regression? There is quite a few of them below.
> It is, but a conscious one. I tried to explain it a bit in the description of the patch, but I can understand this may not have been very clear :)
> This regression is a trade-off to get better diagnostics for the prefetches. We could avoid the regression if we create new operand classes for each shift/extend where it makes the distinction between "both z0.s and z0.d are allowed in this position", and "only z0.d is allowed". That way, the assembler can make the choice what diagnostic to emit. However, I think that would (over)complicate the operand classes.
> Please have a second look at the commit message to see some examples of this trade-off and let me know what you think is the best way forward.
Ah, sorry, I guess I didn't read the description carefully enough!

I didn't count the number of regressions, but this change looks neutral: there are as many regressions in the diagnostics as there are improvements. Perhaps there are slightly more improvements.

The royal approach of course would be to have the best of both worlds (i.e. no regressions), but you write that would require more operands classes. I don't know if that would make things messy or not, but given that the change is neutral, I can live with this change. Perhaps you can wait a day before committing just in case someone can't.



https://reviews.llvm.org/D46688