Currently we are doubling up our test scripts with things like: ; RUN: llvm-as < %s | opt -loop-rotate -disable-output && ; RUN: llvm-as < %s | opt -loop-rotate | llvm-dis | not grep "\[ .tmp224" in order to ensure that any failures early in the pipeline are not causing false positives simply because no output was generated. There are two issues here: 1. Correctness. If we had a mechanism that checked each member of a pipeline for 0 return code, that would be preferable as the test case writer doesn't have to think about that sort of thing happening. By having a mechanism that does this automatically, additional test cases, not covered by the above duplication would be made more robust. 2. Speed. This solution to the problem is coming at the expense of invoking the same pipelines multiple times. While this isn't an issue now while the test suite runs quickly, it will in due time cause issues. The test suite has grown by over 1000 tests in the last year or so. Its likely to accellerate that growth rate for some time to come. This isn't a pressing need, just filing this so we don't forget about it. Reid.
It turns out that the Tcl "exec" command is everything we need: 1. It contains all the pipeline stuff that shells have. 2. It has all the redirection things that we use (and more). 3. It will resolve all Tcl variable names automatically (without the need for us to pattnern match or use funny percent names). 4. We won't need to write a shell script and execute it, Tcl will execute it directly. 5. Each line of execution will check result codes and not continue if there isn't a 0 result code at each step. This means we don't need the guards (which are often forgotten) any more. 6. There is no or little impact to the syntax of the RUN: lines. 7. It will be faster.
Sounds great. CAn you give an example of what the run lines will look like?
They'll look identical (or very nearly identical) to what they are today. We can continue to replace the % variables with their corresponding Tcl variables for a time until all the scripts get converted. For example, instead of %t would could have $tmpFile. For the longer named variables (e.g. %llvmgxx) the only difference would be to change from % to $. And, this is only an optimization as we wouldn't have to invoke a pattern match on each line of the script. Instead the exec command would just do substitution on all the $ named Tcl vars.
Here's a link to the exec command: http://www.tcl.tk/man/tcl/TclCmd/exec.htm
will they support && ?
Where that page says: Macintosh The exec command is not implemented and does not exist under Macintosh. It is patently false. That may have been true of old Mac systems but not Mac OS X. Otherwise the tests wouldn't work (we're using exec today).
&& is not supported, but then its not needed either. Every line will be checked for 0 return code, signals, suspended, posix errors, etc. any kind of failure will fail the test.
ok, so we can say the equiv of: llvm-as < %s | llc | grep abc llvm-as < %s | llc | grep def and it's all good, right?
Yes, exactly. It checks every line, every process.
sounds awesome, when do we switch? ;-)
As soon as I get it working .. most of the way there now. I think the last thing is to support \ at the end of the line
So, this patch: http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20070409/047501.html Yields these results on the Feature tests: Running /proj/llvm/llvm-2/test/Feature/dg.exp ... FAIL: /proj/llvm/llvm-2/test/Feature/globalredefinition3.ll: exit(1) while running: llvm-as < /proj/llvm/llvm-2/test/Feature/globalredefinition3.ll -o /dev/null -f |& grep "Redefinition of global variable named 'B'" llvm-as: <stdin>:6,0: Redefinition of global variable named 'B' of type 'i32' child process exited abnormally === Feature Summary === # of expected passes 45 # of unexpected failures 1 make[1]: *** [check-local] Error 1 make[1]: Leaving directory `/proj/llvm/llvm-2/test' make: *** [check] Error 2 real 0m12.343s user 0m9.754s sys 0m1.799s which is only just slightly faster than the original: Running /proj/llvm/llvm-2/test/Feature/dg.exp ... === Feature Summary === # of expected passes 46 make[1]: Leaving directory `/proj/llvm/llvm-2/test' real 0m12.585s user 0m9.847s sys 0m1.910s I'm not sure what's up with the one test that fails. For some reason it thinks one of the programs is returning a status of 1 and bailing. Either it is and we've caught something we weren't catching before, or the new script is broken. I'll continue working on this tomorrow.
This is working so well that I had to add an "ignore" script to explicitly ignore non-zero result codes in cases where this is appropriate. This is nice because it documents in the test cases situations where we are explicitly ignoring the result code. Otherwise, non-zero causes test failure. To test this a little future, I'm going to convert a few directories and see how the nightly test deals with it on various platforms. If that goes well, I'll slowly migrate the rest of the directories. In the cases I've looked at there are only minor modifications necessary, in these categories: 1. The "2>&1 | grep" idom (to grep error messages) needs to be replaced with "|& grep" because that's what tcl expects. This doesn't happen often and is just using alternate shell syntax (C-Shell). 2. The program is trying to test failure of a tool so we need to insert "ignore" for the failing command(s). 3. The script does something like "grep '<{'" which loses its ' quoting when run so Tcl thinks this is a redirect to {. This is easily taken care of by inserting a \ before the < which is a good idea anyway for any shell. 4. There is actually a bug in the script that has gone undetected. I've found several of these so far.
A few other things I've noticed that need to be changed: 1. grep 'thing' will cause grep to literally look for 'thing' (with the ') which will fail. It just needs to be replaced with: grep thing The reason for this is that tcl is invoking exec directly without any further intepretation of the arguments. I.e. there's no shell her to script away the ' 2. grep 'find a thing' will cause grep to fail because it tries to search files name "a" and "thing'" for "'find". The files don't exist. This is again an effect of not having ' processing. To get this to work it must be changed to grep {find a thing}. In this case the { and } serve to "quote" the pattern into a single argument to grep. So, the question is, do these syntax changes mean death for this PR? Is it too far from shell syntax? Or, do the benefits of having every command in every pipeline checked outweight any issues with syntax? I vote for the latter.
I'm fine with syntax changes. Does this mean we'll have to use "grep find\ a\ thing" ? -Chris
I'm not sure if that would work. OTOH, grep {find a thing} is easier to understand and works.
even better
This has now been implemented with numerous patches. See: http://llvm.org/docs/TestingGuide.html#dgstructure for details on the new syntax for RUN: lines. All test cases needing conversion have been converted.
Turns out that the && at the end of lines were causing some tests to mis report. Tcl interprets & at the end of a command as a command to run in the background. So, there could be some timing related issues with the results generated by such command lines. This has now been corrected.