Using FileCheck in unit tests

When writing MachineFunction unit tests, I find it quite tedious to verify correctness in C++. I would like to use FileCheck in UnitTests because FileCheck is extremely convenient/robust to verify correctness. In order to do so, I moved most of FileCheck’s implementation into a header (Support/FileCheck.h) and updated FileCheck.cpp to use this.

I ran into this while writing some target agnostic Legalization code in GISel where it’s not possible or extremely inconvenient to use llc.
What do others think of this? I would think there might be non GISel uses for FileCheck in unit tests, but I’m not sure.

Here’s a quick patch for this (https://reviews.llvm.org/D48850)

Thanks
Aditya

I had similar gripes with unit testing machine function stuff. I personally would have preferred to create more tests based on a tool like llc rather than pushing more on the unit test side. Anyway I tried to push https://reviews.llvm.org/D48850 in order to extend llc to be more amenable in the situations we want to test, but it ultimately wasn’t accepted.

  • Matthias

I had similar gripes with unit testing machine function stuff. I personally would have preferred to create more tests based on a tool like llc rather than pushing more on the unit test side. Anyway I tried to push https://reviews.llvm.org/D48850 in order to extend llc to be more amenable in the situations we want to test, but it ultimately wasn’t accepted.

  • Matthias

When writing MachineFunction unit tests, I find it quite tedious to verify correctness in C++. I would like to use FileCheck in UnitTests because FileCheck is extremely convenient/robust to verify correctness. In order to do so, I moved most of FileCheck’s implementation into a header (Support/FileCheck.h) and updated FileCheck.cpp to use this.

I ran into this while writing some target agnostic Legalization code in GISel where it’s not possible or extremely inconvenient to use llc.

To elaborate on this, the motivating issue as I understand it is that there’s portions of GlobalISel that will be needed by targets in the future but aren’t used by the current set of targets that support GlobalISel. For example, of the various actions (widenScalar, narrowScalar, Lower, etc.) that can be taken on CTLZ, most of the current GlobalISel targets declare CTLZ legal for almost all the types they encounter in practice. Instead of leaving the other paths untested or unimplemented, you wanted to be able to test those parts of GlobalISel even though we lack a target that would allow us to test it with llc.

Is that right?

What do others think of this? I would think there might be non GISel uses for FileCheck in unit tests, but I’m not sure.

I like the idea of being able to build a mock target to test the areas of GlobalISel that would normally only get very light testing. Being able to test the individual expansions in the combiner is going to be particularly useful as there’s several reasons an individual combine could bitrot over time such there being other combines that overlap many of the same cases, or just simply being a rare case.

I’m not keen on moving the implementation to a header though. If we’re going to make it usable from unittests then I think we should turn it into a library and compile the implementation once.

I had similar gripes with unit testing machine function stuff. I personally would have preferred to create more tests based on a tool like llc rather than pushing more on the unit test side. Anyway I tried to push https://reviews.llvm.org/D48850 in order to extend llc to be more amenable in the situations we want to test, but it ultimately wasn’t accepted.

  • Matthias

When writing MachineFunction unit tests, I find it quite tedious to verify correctness in C++. I would like to use FileCheck in UnitTests because FileCheck is extremely convenient/robust to verify correctness. In order to do so, I moved most of FileCheck’s implementation into a header (Support/FileCheck.h) and updated FileCheck.cpp to use this.

I ran into this while writing some target agnostic Legalization code in GISel where it’s not possible or extremely inconvenient to use llc.

To elaborate on this, the motivating issue as I understand it is that there’s portions of GlobalISel that will be needed by targets in the future but aren’t used by the current set of targets that support GlobalISel. For example, of the various actions (widenScalar, narrowScalar, Lower, etc.) that can be taken on CTLZ, most of the current GlobalISel targets declare CTLZ legal for almost all the types they encounter in practice. Instead of leaving the other paths untested or unimplemented, you wanted to be able to test those parts of GlobalISel even though we lack a target that would allow us to test it with llc.

Yup. That was the main issue. There are also cases where expansion of one instruction changes if some other opcode is legal/custom etc and there’s no way to test both paths/expansions for one target.

Is that right?

What do others think of this? I would think there might be non GISel uses for FileCheck in unit tests, but I’m not sure.

I like the idea of being able to build a mock target to test the areas of GlobalISel that would normally only get very light testing. Being able to test the individual expansions in the combiner is going to be particularly useful as there’s several reasons an individual combine could bitrot over time such there being other combines that overlap many of the same cases, or just simply being a rare case.

Yup. I think it would make testing combines/transformations much easier.

I’m not keen on moving the implementation to a header though. If we’re going to make it usable from unittests then I think we should turn it into a library and compile the implementation once.

I can definitely do that - this was a quick POC.
Thanks

I had similar gripes with unit testing machine function stuff. I personally would have preferred to create more tests based on a tool like llc rather than pushing more on the unit test side. Anyway I tried to push https://reviews.llvm.org/D48850 in order to extend llc to be more amenable in the situations we want to test, but it ultimately wasn’t accepted.

  • Matthias

When writing MachineFunction unit tests, I find it quite tedious to verify correctness in C++. I would like to use FileCheck in UnitTests because FileCheck is extremely convenient/robust to verify correctness. In order to do so, I moved most of FileCheck’s implementation into a header (Support/FileCheck.h) and updated FileCheck.cpp to use this.

I ran into this while writing some target agnostic Legalization code in GISel where it’s not possible or extremely inconvenient to use llc.

To elaborate on this, the motivating issue as I understand it is that there’s portions of GlobalISel that will be needed by targets in the future but aren’t used by the current set of targets that support GlobalISel. For example, of the various actions (widenScalar, narrowScalar, Lower, etc.) that can be taken on CTLZ, most of the current GlobalISel targets declare CTLZ legal for almost all the types they encounter in practice. Instead of leaving the other paths untested or unimplemented, you wanted to be able to test those parts of GlobalISel even though we lack a target that would allow us to test it with llc.

Yup. That was the main issue. There are also cases where expansion of one instruction changes if some other opcode is legal/custom etc and there’s no way to test both paths/expansions for one target.

Is that right?

What do others think of this? I would think there might be non GISel uses for FileCheck in unit tests, but I’m not sure.

I like the idea of being able to build a mock target to test the areas of GlobalISel that would normally only get very light testing. Being able to test the individual expansions in the combiner is going to be particularly useful as there’s several reasons an individual combine could bitrot over time such there being other combines that overlap many of the same cases, or just simply being a rare case.

Yup. I think it would make testing combines/transformations much easier.

  • 1, I can think of a handful of tests in unittests/Transforms/Utils which could be simplified using FileCheck-style checks.

vedant

Aditya Nandakumar via llvm-dev <llvm-dev@lists.llvm.org> writes:

When writing MachineFunction unit tests, I find it quite tedious to
verify correctness in C++. I would like to use FileCheck in UnitTests
because FileCheck is extremely convenient/robust to verify
correctness. In order to do so, I moved most of FileCheck’s
implementation into a header (Support/FileCheck.h) and updated
FileCheck.cpp to use this.

I ran into this while writing some target agnostic Legalization code
in GISel where it’s not possible or extremely inconvenient to use llc.
What do others think of this? I would think there might be non GISel
uses for FileCheck in unit tests, but I’m not sure.

Here’s a quick patch for this (https://reviews.llvm.org/D48850
<https://reviews.llvm.org/D48850>)

Vedant Kumar via llvm-dev <llvm-dev@lists.llvm.org> writes:

+ 1, I can think of a handful of tests in unittests/Transforms/Utils
which could be simplified using FileCheck-style checks.

Sounds like there's some general consensus that this is useful, so I'll
go ahead and start reviewing the patch.

Matthias Braun via llvm-dev <llvm-dev@lists.llvm.org> writes:

I had similar gripes with unit testing machine function stuff. I
personally would have preferred to create more tests based on a tool
like llc rather than pushing more on the unit test side. Anyway I
tried to push https://reviews.llvm.org/D48850 in order to extend llc
to be more amenable in the situations we want to test, but it
ultimately wasn't accepted.

Looks like you put the wrong link here and I'm having trouble finding
the review you're referring to. Do you mind digging it up for me?