On Mon, 18 Jun 2018 at 16:29, Kim Phillips kim.phillips@arm.com wrote:
On Mon, 18 Jun 2018 15:51:34 -0600 Mathieu Poirier mathieu.poirier@linaro.org wrote:
On Mon, 18 Jun 2018 at 15:25, Kim Phillips kim.phillips@arm.com wrote:
On Fri, 15 Jun 2018 16:22:33 -0600 Mathieu Poirier mathieu.poirier@linaro.org wrote:
On Fri, 15 Jun 2018 at 15:54, Kim Phillips kim.phillips@arm.com wrote:
On Fri, 15 Jun 2018 15:38:27 -0600 Mathieu Poirier mathieu.poirier@linaro.org wrote:
On Fri, 15 Jun 2018 at 13:39, Kim Phillips kim.phillips@arm.com wrote: > > On Thu, 14 Jun 2018 14:26:55 -0600 > Mathieu Poirier mathieu.poirier@linaro.org wrote: > > + decoder->instr_count += elem->num_instr_range; > > so.. using your branch, this line doesn't build, native, gcc 6.3, with > O= set: > > util/cs-etm-decoder/cs-etm-decoder.c: In function ‘cs_etm_decoder__buffer_range’: > util/cs-etm-decoder/cs-etm-decoder.c:351:30: error: ‘ocsd_generic_trace_elem {aka const struct _ocsd_generic_trace_elem}’ has no member named ‘num_instr_range’ > decoder->instr_count += elem->num_instr_range; > ^~ > mv: cannot stat '/nfsroot/linux/tools/arm64-perf/util/cs-etm-decoder/.cs-etm-decoder.o.tmp': No such file or directory > linux/tools/build/Makefile.build:96: recipe for target 'linux/tools/arm64-perf/util/cs-etm-decoder/cs-etm-decoder.o' failed > > Did the line adding num_instr_range get somehow lost on its way into > the patch?
That is not the case nor is it a compiler version problem. I completely forgot to highlight this set need to be compiled with the latest version of the openCSD library where the decoder counts the amount of instruction present in a range.
The code ought to be able to handle, and remain backwards compatible with an older version of the library.
Once again I do not agree. It is only normal that developers take advantage of new functionality available in a library and it happens all the time.
Not when it breaks the build for the older version of the library. Where do you see that happening with the upstream perf tool?
Any library embedded in the perf tools (as openCSD is) works exactly
The openCSD library is not embedded in the perf tool source tree. libopencsd is a standalone library: it's available by itself, and tools like perf can optionally detect and use it.
the same way. If people are doing development work they are expected to work with the latest revision of the library they are using. For
Not necessarily: Hello world doesn't require the latest C library.
Try using the latest features of the C library on any old system, things will break.
distributions the linux tools package picks up the latest revision.
libopencsd will only be updated upon distribution maintainer and user update action. Meanwhile, perf should continue to build no matter what.
Distribution perf tools and the openCSD library are always in sync, that's not a problem.
When downloading the latest revision of the perf tools, it is normal that the latest features may not be present in the libraries available on the system - this is true for any library. OpenCSD provides flags to compile with an alternate (i.e newer) revision of the library exactly for those situations. Those are well documented and I've used them many times.
I think Rob was looking at a way to warn users about an openCSD that is too old.
That's fine, but perf should still build without crippling users with older versions of libraries installed.
And how do we do this? We add #ifdefs everywhere? Every tool I've
That would work.
That is impossible to maintain, which is why people simply don't do that.
recompiled needs a specific version of a library... This standard practice.
Not in the upstream perf tool.
Then we agree to disagree.
Kim