[PATCH] D34362: [LNT] Support for different DataSet usage in Polybench for "lnt runtest nt"

Utpal Bora via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Sun Jun 25 23:30:53 PDT 2017


cs14mtech11017 added a comment.

In https://reviews.llvm.org/D34362#788827, @kristof.beyls wrote:

> In https://reviews.llvm.org/D34362#788285, @Meinersbur wrote:
>
> > I think `nt.py` is deprecated, and meant to be replaced by `test_suite.py`.
> >
> > Maybe someone who already contributed to lnt should be necessary for acceptance.
>
>
> A few of the most active developers on test-suite and lnt at least think it's better to use the (new) CMake+lit-based system to drive the test-suite rather than the (old) Makefile-based system. 'nt.py' in LNT is a layer around the Makefile-based system, "test_suite.py" is a layer around the CMake+lit-based system. I guess you could say that makes nt.py "deprecated".
>
> Anyway, on the patch itself here: I don't really like adding ever more separate command line options for specifying different data sizes available in a test suite. Wouldn't it be better to introduce a command line argument spelled e.g. "--datasize=mini|small|standard|large|extra_large"? We could then also hopefully deprecate "--small", "--large", "--spec-with-ref", etc., making them aliases for the new command line option for backwards compatibility. Note that I haven't thought about what the best name would be for this option. Maybe "inputsize" is more in line with common terminology across benchmarks? Although it may be hard to guess from the "inputsize" exactly what it means without further explanation either...


I agree with you. I was planning to add such a flag for all the sizes  "--datasize=mini|small|standard|large|extra_large". Did not do it for compatibility. Thanks for the suggestion about aliasing for backward compatibility. If we agree on adding such an option, I think "--testdatasize" will be appropriate. And I would argue about using a common flag like this one for all for all data sizes instead of using "--make-param" flag as pointed out by @cmatthews.

> Also, please in the future upload diffs with more context, see http://llvm.org/docs/Phabricator.html#requesting-a-review-via-the-web-interface.

Ohh I missed that.


Repository:
  rL LLVM

https://reviews.llvm.org/D34362





More information about the llvm-commits mailing list