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

[Hal Finkel via Phabricator via llvm-commits]

hfinkel added a comment.

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.


Repository:
  rT test-suite

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

https://reviews.llvm.org/D61116