<div dir="ltr">I'm pretty change averse - and am in this case (but doesn't mean other folks won't be in favor and doesn't mean it isn't worth trying, etc - but if it were up to me, at the moment, I'd decline)<br><br>To take one point from the phab review (to try to keep the discussion here rather than there - might be worth a note on the phab review to discourage commenting there so the discussion doesn't get spread through different threads):<br><br>> <span style="color:rgb(0,0,0);font-family:"Segoe UI","Segoe UI Emoji","Segoe UI Symbol",Lato,"Helvetica Neue",Helvetica,Arial,sans-serif;font-size:13px">Because of all of the above, maintenance of all the regression tests is a nightmare. I expect it to be a serious issue for introducing opaque pointers. My prediction is that we will have a typed-pointer command line flag to not have to require updating all the write-only regression tests.<br><br>Actually I already did a lot of work with the initial patches years ago for opaque pointers (that added explicit types to gep/store/etc) and used (& provided in the commit messages) python scripts to migrate all the tests, both the IR itself and the CHECK text. This is probably more readily automatable than a more free-form C++ based checking proposed here.<br><br>That said, it sounds like the proposal is a lot closer to the GUnit tests, and if this testing strategy is valuable, it seems like it could be mostly achieved by adding some API utilities (like the one you proposed in the patch) to make it more convenient to run optimization passes in a GUnit test. It doesn't seem to me like an #ifdef based approach to embedding IR in C++ would result in particularly more manageable/formattable code than a raw string. Perhaps the proposed improvements could be used to reduce/remove the cost of adding new GUnit tests/the need to touch CMake files/etc. (though I'd worry about the divergence of where optimization tests are written - as unit tests or as lit/FileCheck tests - that doesn't mean experimentation isn't worthwhile, but I think it'd require a pretty compelling justification to propose a replacement to the FileCheck approach (& perhaps a timeline for an experiment before either removing it, or deciding that it is the intended future state) - and if it's not a replacement, then I think we'd need to discuss what sort of situations this new thing is suitable and what FileCheck should be used for going forward)<br><br>- Dave</span></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Tue, Jun 23, 2020 at 6:34 PM Michael Kruse via llvm-dev <<a href="mailto:llvm-dev@lists.llvm.org">llvm-dev@lists.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Hello LLVM community,<br>
<br>
For testing IR passes, LLVM currently has two kinds of tests:<br>
1. regression tests (in llvm/test); .ll files invoking opt, and<br>
matching its text output using FileCheck.<br>
2. unittests (in llvm/unittests); Google tests containing the IR as a<br>
string, constructing a pass pipeline, and inspecting the output using<br>
code.<br>
<br>
I propose to add an additional kind of test, which I call "compiled<br>
regression test", combining the advantages of the two. A test is a<br>
single .cxx file of the general structure below that can be dumped<br>
into the llvm/test directory. I am not proposing to replace FileCheck,<br>
but in a lot of cases, domain-specific verifiers can be more powerful<br>
(e.g. verify-uselistorder or `clang -verify`).<br>
<br>
#ifdef IR<br>
define void @func() {<br>
entry:<br>
ret<br>
}<br>
#else /* IR */<br>
#include "compiledtestboilerplate.h"<br>
TEST(TestSuiteName, TestName) {<br>
unique_ptr<Module> Output = run_opt(__FILE__, "IR",<br>
"-passes=loop-vectorize");<br>
/* Check Output */<br>
}<br>
#endif /* IR */<br>
<br>
That is, input IR and check code are in the same file. The run_opt<br>
command is a replica of main() from the opt tool, so any command line<br>
arguments (passes with legacy or new passmanager, cl::opt options,<br>
etc.) can be passed. It also makes converting existing tests simpler.<br>
<br>
The top-level structure is C++ (i.e. the LLVM-IR is removed by the<br>
preprocessor) and compiled with cmake. This allows a<br>
compile_commands.json to be created such that refactoring tools,<br>
clang-tidy, and clang-format can easily be applied on the code. The<br>
second argument to run_opt is the preprocessor directive for the IR<br>
such that multiple IR modules can be embedded into the file.<br>
<br>
Such tests can be compiled in two modes: Either within the LLVM<br>
project, or as an external subproject using llvm_ExternalProject_Add.<br>
The former has the disadvantage that new .cxx files dumped into the<br>
test folder are not recognized until the next cmake run, unless the<br>
CONFIGURE_DEPENDS option is used. I found this adds seconds to each<br>
invocation of ninja which I considered a dealbreaker. The external<br>
project searched for tests every time, but is only invoked in a<br>
check-llvm run, no different than llvm-lit. It uses CMake's<br>
find_package to build against the main project's results (which<br>
currently we do not have tests for) and could also be compiled in<br>
debug mode while LLVM itself is compiled in release mode.<br>
<br>
The checks themselves can be any of gtest's ASSERT/EXPECT macros, but<br>
for common test idioms I suggest to add custom macros, such as<br>
<br>
ASSERT_ALL_OF(InstList, !isa<VectorType>(I->getType()));<br>
<br>
which on failure prints the instruction that does not return a vector.<br>
Try that with FileCheck. PattenMatch.h from InstCombine can be used as<br>
well. Structural comparison with a reference output could also be<br>
possible (like clang-diff,<br>
[llvm-canon](<a href="http://llvm.org/devmtg/2019-10/talk-abstracts.html#tech12" rel="noreferrer" target="_blank">http://llvm.org/devmtg/2019-10/talk-abstracts.html#tech12</a>),<br>
<a href="https://reviews.llvm.org/D80916" rel="noreferrer" target="_blank">https://reviews.llvm.org/D80916</a>).<br>
<br>
Some additional tooling could be helpful:<br>
<br>
* A test file creator, taking some IR, wrapping it into the above<br>
structure, and write it into the test directory.<br>
* A tool for extracting and updating (running opt) the IR inside the<br>
#ifdef, if not even add this functionality to opt itself. This is the<br>
main reason to not just the IR inside a string.<br>
<br>
A Work-in-Progress differential and what it improves over FileCheck<br>
and unittests is available here: <a href="https://reviews.llvm.org/D82426" rel="noreferrer" target="_blank">https://reviews.llvm.org/D82426</a><br>
<br>
Any kind of feedback welcome.<br>
_______________________________________________<br>
LLVM Developers mailing list<br>
<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a><br>
<a href="https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev" rel="noreferrer" target="_blank">https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev</a><br>
</blockquote></div>