[PATCH] D24548: Add AArch64 unit tests

[Diana Picus via llvm-commits]

rovka added a comment.

Hi. Thanks for looking :)

In https://reviews.llvm.org/D24548#545770, @rengolin wrote:

> I'm guessing that the MIR snippet you have in `runChecks()` is the bare minimum to make it parse, in which case, any sequence of machine instructions would work with that pattern. For now, that being inside an anonymous namespace in the AArch64 part of the tests is ok, but if more targets start using this pattern (which I think it's a good thing), we should be moving this up somewhere. Not in this patch, but maybe a comment would be good.


It's the bare minimum to run these 3 tests, and it's probably good enough for any instruction-size tests. It used to be even smaller before I had to add the global for the TLS test, so it's not impossible for other tests to have other needs as well (e.g. tests that need more than one basic block or more than one function would probably rather not have this particular MIR stub). I think it needs to evolve a bit before being extracted. I'll try to think of a comment along those lines.


================
Comment at: unittests/Target/AArch64/InstSizes.cpp:88
@@ +87,3 @@
+              EXPECT_EQ(16u, II.getInstSizeInBytes(*I));
+
+              ++I;
----------------
rengolin wrote:
> why the extra space?
Erm, it just felt like a natural way to separate the 2 checks. But I guess with everything else so compact it may look a bit awkward. I'll remove it before committing.


https://reviews.llvm.org/D24548