llvm dev - Jan 2017 - Removed a call to EmitXRayTable() from ARMAsmPrinter

Sharing with the mailing list... Please, see below.

On 9 January 2017 at 23:45, Serge Rogatch <serge.rogatch at gmail.com>
wrote:
> Hi Dean,
>
> I have seen that you removed the following code from ARMAsmPrinter.cpp in
> revision 290858:
>   // Emit the XRay table for this function.
>   EmitXRayTable();
>
> Was this done by mistake or on purpose?
>
> Without this call, xray_instr_map gets empty in the executable, so that
> XRay doesn't patch anything. While implementing the tail call handling,
I
> am reverting this change (I mean, just returning the call to
> emitXRayTable() where it was), or are there reasons I shouldn't?
>
> Cheers,
> Serge
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20170109/f4460f40/attachment.html>

On 9 January 2017 at 20:47, Serge Rogatch via llvm-dev
<llvm-dev at lists.llvm.org> wrote:
>> Without this call, xray_instr_map gets empty in the executable, so that
>> XRay doesn't patch anything. While implementing the tail call
handling, I am
>> reverting this change (I mean, just returning the call to
emitXRayTable()
>> where it was), or are there reasons I shouldn't?
Any reason why no tests were harmed in the process? Maybe we're not
testing as thorough as we thought?

--renato

Hi Renato,

As far as I understand, such issues should be caught by the tests in
compiler-rt/test/xray/TestCases/Linux.
I found the following lines in compiler-rt/test/xray/lit.cfg that seem to
disable the tests for non-64-bit targets:

if config.host_os not in ['Linux'] or
config.host_arch.find('64') == -1:
  config.unsupported = True

@Serge: You will need to change this condition to enable the tests for ARM.

Oleg

________________________________________
From: Renato Golin <renato.golin at linaro.org>
Sent: Monday, January 9, 2017 11:50 PM
To: Serge Rogatch
Cc: Dean Michael Berris; LLVM Developers; Oleg Ranevskyy
Subject: Re: [llvm-dev] Removed a call to EmitXRayTable() from ARMAsmPrinter

On 9 January 2017 at 20:47, Serge Rogatch via llvm-dev
<llvm-dev at lists.llvm.org> wrote:
>> Without this call, xray_instr_map gets empty in the executable, so that
>> XRay doesn't patch anything. While implementing the tail call
handling, I am
>> reverting this change (I mean, just returning the call to
emitXRayTable()
>> where it was), or are there reasons I shouldn't?
Any reason why no tests were harmed in the process? Maybe we're not
testing as thorough as we thought?

--renato