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

[Roman Lebedev via Phabricator via llvm-commits]

lebedev.ri added a comment.

In D61116#1484567 <https://reviews.llvm.org/D61116#1484567>, @hfinkel wrote:

> In D61116#1483718 <https://reviews.llvm.org/D61116#1483718>, @lebedev.ri wrote:
>
> > In D61116#1483599 <https://reviews.llvm.org/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://reviews.llvm.org/rL345166> / https://llvm.org/docs/Proposals/TestSuite.html#rawspeed

In D61116#1484741 <https://reviews.llvm.org/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 :/


Repository:
  rT test-suite

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D61116/new/

https://reviews.llvm.org/D61116