[test-suite] MultiSource/Benchmarks/DOE-ProxyApps-C/RSBench: change *CMAKE* target name only

[lebedev.ri]

Summary

That benchmark produces a binary named rsbench, and creates cmake target with name rsbench.
All that is good. But unfortunately, 'rsbench' is a very short abbreviation.

In particular, it just so happened that i have a main benchmark with the exact same name:
https://github.com/darktable-org/rawspeed/blob/fd5f06086ff323e0a2bdc1e2234dae6ae4e32532/src/utilities/rsbench/CMakeLists.txt#L1
While i'm not sure if it will be possible to integrate that project into test-suite proper,
i'd like to use it at least for local testing. But that is problematic if there is name clashes.

While i realize that it is plain impossible to avoid clashes in general, i'm hoping this particular situation can be solved,
In particular, the problem is *ONLY* the CMake target name. The binary name does NOT matter.
Therefore, how about a change from cmake target name of rsbench to doe-rsbench,
while ensuring the binary name stays the same?

[hfinkel]

While I'm sympathetic, we have a lot of benchmark names which are short and could conflict with other things. We have xsbench too. Either we should solve this uniformly, or not at all. You can always carry local patches for name conflicts with out-of-tree things.

[lebedev.ri]

In D61116#1483599, @hfinkel wrote:

While I'm sympathetic, we have a lot of benchmark names which are short and could conflict with other things.
We have xsbench too.

Sure, yes. But handling conflicts as they appears isn't really an invalid strategy, either.
This *only* changes the cmake target name, not the binary name, so this should be virtually unnoticeable,
unless one directly cares about that cmake target name that is of course.

Either we should solve this uniformly, or not at all.

Can you define the scope of uniformly? Every [a-zA-Z][a-zA-Z0-9]bench?

You can always carry local patches for name conflicts with out-of-tree things.

I *could* but i'm trying to avoid that.

[hfinkel]

In D61116#1483718, @lebedev.ri wrote:

In D61116#1483599, @hfinkel wrote:

While I'm sympathetic, we have a lot of benchmark names which are short and could conflict with other things.
We have xsbench too.

Sure, yes. But handling conflicts as they appears isn't really an invalid strategy, either.
This *only* changes the cmake target name, not the binary name, so this should be virtually unnoticeable,
unless one directly cares about that cmake target name that is of course.

I'm aware, but this is also a slippery slope. You have the upstream project carrying a patch to avoid an uncommon naming conflict with an out-of-tree enhancement. That's something that we've tried, in general, not to do.

Either we should solve this uniformly, or not at all.

Can you define the scope of uniformly? Every [a-zA-Z][a-zA-Z0-9]bench?

No, I mean making all of the test suite targets have some naming convention that makes conflict less likely. Maybe adding llvm-ts- to all of them, for example. Maybe you could build that into the llvm_multisource macro and thus solve the problem for everything at once.

You can always carry local patches for name conflicts with out-of-tree things.

I *could* but i'm trying to avoid that.

I realize. But the way you avoid that is to contribute your benchmark to the test suite, thus making the naming conflict something we figure out how to resolve during code review.

[Meinersbur]

llvm_multisource has an option "PREFIX" that can be used to disambiguate target names. However, in my experience when using it collecting metrics does not work as reliable with this options.

Why not changing the name of rsbench downstream? Changing upstream to solve downstream conflicts seems to be the wrong direction.

MultiSource/Benchmarks/DOE-ProxyApps-C/RSBench/CMakeLists.txt

4

[serious] Please add the reason why it has to have this name.

[lebedev.ri]

In D61116#1484567, @hfinkel wrote:

In D61116#1483718, @lebedev.ri wrote:

In D61116#1483599, @hfinkel wrote:

While I'm sympathetic, we have a lot of benchmark names which are short and could conflict with other things.
We have xsbench too.

Sure, yes. But handling conflicts as they appears isn't really an invalid strategy, either.
This *only* changes the cmake target name, not the binary name, so this should be virtually unnoticeable,
unless one directly cares about that cmake target name that is of course.

I'm aware, but this is also a slippery slope. You have the upstream project carrying a patch to avoid an uncommon naming conflict with an out-of-tree enhancement. That's something that we've tried, in general, not to do.

Either we should solve this uniformly, or not at all.

Can you define the scope of uniformly? Every [a-zA-Z][a-zA-Z0-9]bench?

No, I mean making all of the test suite targets have some naming convention that makes conflict less likely. Maybe adding llvm-ts- to all of them, for example. Maybe you could build that into the llvm_multisource macro and thus solve the problem for everything at once.

You can always carry local patches for name conflicts with out-of-tree things.

I *could* but i'm trying to avoid that.

I realize. But the way you avoid that is to contribute your benchmark to the test suite, thus making the naming conflict something we figure out how to resolve during code review.

rL345166 / https://llvm.org/docs/Proposals/TestSuite.html#rawspeed

In D61116#1484741, @Meinersbur wrote:

llvm_multisource has an option "PREFIX" that can be used to disambiguate target names. However, in my experience when using it collecting metrics does not work as reliable with this options.

Why not changing the name of rsbench downstream? Changing upstream to solve downstream conflicts seems to be the wrong direction.

The main thing is, i'd like for it to be seamlessly integrateable into testsuite, with *no* diff on either side.
That is:

• I don't want to change my rsbench in upstream, because i *quite* often have to type it, manually.
• I would prefer not to change my rsbench (when the time comes to actually upstream it into testsuite), because then that will be a diff from upstream.
• I don't really see any way to carry this (MultiSource/Benchmarks/DOE-ProxyApps-C/RSBench/CMakeLists.txt) patch.

So the question, as i have seen it, is whether test-suite 'consumers' often have to manually type the rsbench for the current DOE-ProxyApps-C/RSBench ?
I have guessed no, and thus it seemed better to rename *it*.

It is indeed quite random name clash :/

[hfinkel]

In D61116#1484835, @lebedev.ri wrote:

In D61116#1484567, @hfinkel wrote:

In D61116#1483718, @lebedev.ri wrote:

In D61116#1483599, @hfinkel wrote:

While I'm sympathetic, we have a lot of benchmark names which are short and could conflict with other things.
We have xsbench too.

Sure, yes. But handling conflicts as they appears isn't really an invalid strategy, either.
This *only* changes the cmake target name, not the binary name, so this should be virtually unnoticeable,
unless one directly cares about that cmake target name that is of course.

I'm aware, but this is also a slippery slope. You have the upstream project carrying a patch to avoid an uncommon naming conflict with an out-of-tree enhancement. That's something that we've tried, in general, not to do.

Either we should solve this uniformly, or not at all.

Can you define the scope of uniformly? Every [a-zA-Z][a-zA-Z0-9]bench?

No, I mean making all of the test suite targets have some naming convention that makes conflict less likely. Maybe adding llvm-ts- to all of them, for example. Maybe you could build that into the llvm_multisource macro and thus solve the problem for everything at once.

You can always carry local patches for name conflicts with out-of-tree things.

I *could* but i'm trying to avoid that.

I realize. But the way you avoid that is to contribute your benchmark to the test suite, thus making the naming conflict something we figure out how to resolve during code review.

rL345166 / https://llvm.org/docs/Proposals/TestSuite.html#rawspeed

In D61116#1484741, @Meinersbur wrote:

llvm_multisource has an option "PREFIX" that can be used to disambiguate target names. However, in my experience when using it collecting metrics does not work as reliable with this options.

Why not changing the name of rsbench downstream? Changing upstream to solve downstream conflicts seems to be the wrong direction.

The main thing is, i'd like for it to be seamlessly integrateable into testsuite, with *no* diff on either side.
That is:

• I don't want to change my rsbench in upstream, because i *quite* often have to type it, manually.
• I would prefer not to change my rsbench (when the time comes to actually upstream it into testsuite), because then that will be a diff from upstream.
• I don't really see any way to carry this (MultiSource/Benchmarks/DOE-ProxyApps-C/RSBench/CMakeLists.txt) patch.

So the question, as i have seen it, is whether test-suite 'consumers' often have to manually type the rsbench for the current DOE-ProxyApps-C/RSBench ?
I have guessed no, and thus it seemed better to rename *it*.

It is indeed quite random name clash :/

I understand, but we all need to deal with our own down-stream name clashes. If this problem is worth solving, and I'm happy to believe that it is, then it's not worth solving just for this instance. We cannot have a project where naming choices for anything are restricted by essentially-random downstream conflicts. I realize that this seems like a small change, but it is just not reasonable to make this change for this one case. As I said above, if you want to make the whole issue of conflicts less likely by changing how llvm_multisource defines its targets, then I'm supportive. Just for this one name, I think this is a bad precedent.

MultiSource/Benchmarks/DOE-ProxyApps-C/RSBench/CMakeLists.txt

 list(APPEND LDFLAGS -lm)
 list(APPEND CFLAGS -std=gnu99)
 set(RUN_OPTIONS -s small -l 100000 -p 1000 -w 1000)
-llvm_multisource(rsbench)
+# WARNING: *please* keep the cmake target name as doe-rsbench.
+llvm_multisource(doe-rsbench)
+set_target_properties(doe-rsbench PROPERTIES OUTPUT_NAME "rsbench")