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

[• post.kadirselcuk]

Summary

Polybech supports five datasets (mini, small, standard, large and extralarge) whereas LNT nt test supports selection of only two dataset (small and large).
Added support for all five datasets of Polybench in LLVM test suite for nt test.

Depends on [D34363]

[Meinersbur]

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.

[kristof.beyls]

In 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...

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

[grosser]

Thanks Kristof for jumping in. I think your comments are clearly pointing out the right direction. @cs14mtech11017 , I suggest you follow these suggestions.

[MatzeB]

• Indeed, active test-suite development is done in the cmake build (test_suite.py in lnt). You may still change nt.py but landing features exclusively there would be bad.
• It seems the llvm test-suite today only checks for SMALL_PROBLEM_SIZE, occasionally LARGE_PROBLEM_SIZE and for SPEC you can choose the size manually. Do you plan to change the test-suite for the new sizes? We should probably do further discussions there.
• Do you plan to set any rules/expectations on those new sizes? (Something like "mini" finished in half the time than "small")
• Out of interest: Why do you need finer control over the problem sizes?

[cmatthews]

It makes more sense to me to be using the --make-param flag to pass a test specific configuration options. If you want to add all these size classes, all the tests should support them, or have them mapped back to nearest size the tests can handle.

[cs14mtech11017]

In 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.

Ok. I will look into test_suite.py and update the patch.

[cs14mtech11017]

In D34362#788827, @kristof.beyls wrote:

In 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.

[cs14mtech11017]

In D34362#789152, @MatzeB wrote:

• Indeed, active test-suite development is done in the cmake build (test_suite.py in lnt). You may still change nt.py but landing features exclusively there would be bad.

Thanks for letting me know. Will update for both.

• It seems the llvm test-suite today only checks for SMALL_PROBLEM_SIZE, occasionally LARGE_PROBLEM_SIZE and for SPEC you can choose the size manually. Do you plan to change the test-suite for the new sizes? We should probably do further discussions there.

I added these options only for Polybench. Have not inspected the other benchmarks. I would like to discuss how we can map these sizes for other benchmarks? One we agree upon something, I can make the changes for the test-suite.

• Do you plan to set any rules/expectations on those new sizes? (Something like "mini" finished in half the time than "small")

I have not thought about it yet.

• Out of interest: Why do you need finer control over the problem sizes?

I am working with Polybench, which supports multiple input dataset sizes. I did these changes for my experiments and thought it would be better to upload the changes for the community.

[cs14mtech11017]

In D34362#789280, @cmatthews wrote:

It makes more sense to me to be using the --make-param flag to pass a test specific configuration options. If you want to add all these size classes, all the tests should support them, or have them mapped back to nearest size the tests can handle.

I feel having a common flag for test data sizes is better than having to pass them as test specific flag. As you suggested, we can map them back to nearest data sizes for the other tests. I was working with Polybench, so did not look into other tests. Will proceed with it if everyone agrees on adding a flag like "--testdatasizes=mini|small|medium|large|extra_large" and aliasing --small and --large for backward compatibility.

[MatzeB]

In D34362#790215, @cs14mtech11017 wrote:

In D34362#789280, @cmatthews wrote:

It makes more sense to me to be using the --make-param flag to pass a test specific configuration options. If you want to add all these size classes, all the tests should support them, or have them mapped back to nearest size the tests can handle.

I feel having a common flag for test data sizes is better than having to pass them as test specific flag. As you suggested, we can map them back to nearest data sizes for the other tests. I was working with Polybench, so did not look into other tests. Will proceed with it if everyone agrees on adding a flag like "--testdatasizes=mini|small|medium|large|extra_large" and aliasing --small and --large for backward compatibility.

Look at SPEC for example: It has 3 different sizes: test, train and ref. You can describe them roughly as:

• test: "As fast as posisble, not useful as a benchmark but they touch enough code paths so that they can give you a quick way to test for correctness"
• train: "Somewhat realistic but smaller inputs, useful to produce data for PGO optimizations. The important aspect here is that the data is different enough from ref, so we don't overfit the code because training and reference data were the same"
• ref: "Larger inputs running for a longer time producing stable numbers".

So there is some semantics and intended uses here that is captured fine with "test", "train" and "ref". Mapping this to some generic terms like "small", "medium", "large" that just map to sizes would be a loss IMO.

[kristof.beyls]

In D34362#792614, @MatzeB wrote:

In D34362#790215, @cs14mtech11017 wrote:

In D34362#789280, @cmatthews wrote:

It makes more sense to me to be using the --make-param flag to pass a test specific configuration options. If you want to add all these size classes, all the tests should support them, or have them mapped back to nearest size the tests can handle.

I feel having a common flag for test data sizes is better than having to pass them as test specific flag. As you suggested, we can map them back to nearest data sizes for the other tests. I was working with Polybench, so did not look into other tests. Will proceed with it if everyone agrees on adding a flag like "--testdatasizes=mini|small|medium|large|extra_large" and aliasing --small and --large for backward compatibility.

Look at SPEC for example: It has 3 different sizes: test, train and ref. You can describe them roughly as:

• test: "As fast as posisble, not useful as a benchmark but they touch enough code paths so that they can give you a quick way to test for correctness"
• train: "Somewhat realistic but smaller inputs, useful to produce data for PGO optimizations. The important aspect here is that the data is different enough from ref, so we don't overfit the code because training and reference data were the same"
• ref: "Larger inputs running for a longer time producing stable numbers".

So there is some semantics and intended uses here that is captured fine with "test", "train" and "ref". Mapping this to some generic terms like "small", "medium", "large" that just map to sizes would be a loss IMO.

Right, that's a fair point.

FWIW, "lnt runtest test-suite" doesn't have --small or --large or --spec-with-ref command line options; but it does have a --test-size option, seemingly with the same intention as the "--testdatasizes" command line option under discussion here.
That being said, that "--test-size" option currently doesn't seem to actually be doing anything...

I can see that in different circumstances, you'd either want the generic flag vs the SPEC-specific flags. For example:

• Assuming you want to run as many of the programs in the test-suite (with externals plugged in) as possible, you'd probably want to use the generic flag that maps to "small", "medium", "large" to control for how long the benchmark runs. Or how much memory it could end up using. Or maybe some other aspect of program scalability. I don't think I've seen small vs large defined anywhere particularly well...
• If you're only going to run the SPEC benchmarks, you'd probably want to be able to state to run "test", "train" or "ref", rather than "small", etc.
• Of course if you want to retain the original terms and meanings for data size used during benchmarking for SPEC, you can also argue you could want that for other benchmarks like Polybench.

So, in summary, I'm not sure what the conclusion here should be.
If we had a reasonable definition of what "small" or "large" meant, or intended to mean, that would make this decision easier.
For now, I think it's fine for --small and --large to map to traditional "small" and "large" semantics for the test-suite.
Maybe we should refrain from introducing further command line options in LNT for all possible benchmark-specific sizes and indeed set them using --cmake-define/--make-param, to avoid a further explosion of lnt runtest command line options?

[Meinersbur]

What happened?

lnt/tests/nt.py

             make_variables['DISABLE_LTO'] = '1'
         if self.test_llcbeta:
             make_variables['ENABLE_LLCBETA'] = '1'
-        if self.test_small:
+        if self.test_mini:
+            if self.test_small or self.test_standard or  self.test_large or self.test_extralarge:
+                fatal('the size options are mutually exclusive')
+            make_variables['MINI_PROBLEM_SIZE'] = '1'
+        elif self.test_small:
+            if self.test_mini or self.test_standard or  self.test_large or self.test_extralarge:
+                fatal('the size options are mutually exclusive')
             make_variables['SMALL_PROBLEM_SIZE'] = '1'
-        if self.test_large:
-            if self.test_small:
-                fatal('the --small and --large options are mutually exclusive')
+        elif self.test_standard:
+            if self.test_mini or self.test_small or self.test_large or self.test_extralarge:
+                fatal('the size options are mutually exclusive')
+            make_variables['STANDARD_PROBLEM_SIZE'] = '1'
+        elif self.test_large:
+            if self.test_mini or self.test_small or self.test_standard or self.test_extralarge:
+                fatal('the size options are mutually exclusive')
             make_variables['LARGE_PROBLEM_SIZE'] = '1'
+        elif self.test_extralarge:
+            if self.test_mini or self.test_small or self.test_standard or self.test_large:
+                fatal('the size options are mutually exclusive')
+            make_variables['EXTRALARGE_PROBLEM_SIZE'] = '1'
+        else: 
+            make_variables['STANDARD_PROBLEM_SIZE'] = '1'
         if self.test_benchmarking_only:
             make_variables['BENCHMARKING_ONLY'] = '1'
         if self.test_integrated_as:

                          help="Disable use of link-time optimization",
                          action="store_false", default=True)
 
+        group.add_option("", "--mini", dest="test_mini",
+                         help="Use mini test inputs and disable other tests",
+                         action="store_true", default=False)
         group.add_option("", "--small", dest="test_small",
-                         help="Use smaller test inputs and disable large tests",
+                         help="Use smaller test inputs and disable other tests",
                          action="store_true", default=False)
+        group.add_option("", "--standard", dest="test_standard",
+                         help="Use standard test inputs and disable other tests",
+                         action="store_true", default=False)
         group.add_option("", "--large", dest="test_large",
-                         help="Use larger test inputs",
+                         help="Use large test inputs and disable other tests",
                          action="store_true", default=False)
+        group.add_option("", "--extralarge", dest="test_extralarge",
+                         help="Use extralarge test inputs and disable other tests",
+                         action="store_true", default=False)
         group.add_option("", "--spec-with-ref", dest="test_spec_ref",
                          help="Use reference test inputs for SPEC.  "
                          "This is currently experimental",