[PATCH] D39091: [AArch64][SVE] Asm: Add support for (ADD|SUB)_ZZZ

[Alex Bradbury via Phabricator via llvm-commits]

asb added inline comments.


================
Comment at: test/MC/AArch64/SVE/assembler_tests/add.s:5
+// CHECK: add     z31.s, z31.s, z31.s // encoding: [0xff,0x03,0xbf,0x04]
+// CHECK-ERROR: {{(unexpected token in argument list)|(invalid operand for instruction)|(invalid sve vector register)|(unexpected floating point literal)|(expected ']' in brackets expression)|(instruction requires: sve)|(vector register expected)|(immediate must be an integer in range \[-128, 127\] or a multiple of 256 in range \[-32768, 32512\])}}
+// CHECK-ERROR-NEXT: 00000100-10111111-00000011-11111111
----------------
sdesmalen wrote:
> asb wrote:
> > Is this really better than just matching the specific error message? If one of the messages in this list changes in the future, you either have churn across every CHECK-ERROR line in the SVE test cases, or else the CHECK-ERROR lines aren't all updated and you're left with outdated checks. You might also want to know if the error message for a given line changes between one of these unexpectedly.
> > 
> > Of course if FileCheck let you assign a pattern to a variable and reuse it, the trade-off would be different...
> One of the things I forgot to mention is that these tests are all auto-generated from our architecture spec. If one of the error messages changes, we can update the generator and create a new set of tests. Even if we had more specific messages, I think we still have to update quite a bunch of tests anyway, but I agree with you that testing for a specific error message makes more sense (and indeed we could pick up on changes between the checked error messages). When we did the assembler/disassembler work, it covered the cases well enough.
> 
> That said, I'm happy to make changes to the tests. It is actually a good topic to get feedback on, and we don't necessarily need to stick with the auto-generated approach.
> 
> For now though I would rather collect some more input on how the tests should look before making the changes in each patch individually, for the following reasons:
> 1. If there are some 'batch' changes we want to make, we can easily regenerate the tests.
> 2. I have currently split up the assembler/disassembler into many, many small patches, including tests. If I would re-generate the tests now, I'd need to fix each patch individually which is a pain :)
> 
> When we agree how we want the tests to look and most of the patches are in, I can easily create patches for the tests alone. Does that approach make sense?
The only problem with auto-generated tests is that if the generator isn't publicly available it's difficult for people who don't have access to extend, refactor or otherwise improve the tests. Auto-generated tests are great, but perhaps you can make it so the test is generated in a form that's closer to how a human would write it?

Avoiding extra work for yourself while the patches are discussed certainly sounds reasonable to me!


https://reviews.llvm.org/D39091