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

Why there are no tests that would break in this case?

On Mon, Jan 9, 2017 at 8:32 PM, Dean Michael Berris via llvm-dev
<llvm-dev at lists.llvm.org> wrote:
> Oops -- sorry about that, this is definitely unintended. Adding in Martin
who made the change.
>
> I'm happy to review changes to fix this, or if you don't get to it
first I can drive by that part of the code and try and fix it myself. I
unfortunately don't have easy access to arm machines so it is a little hard
for me to try and fix these things proactively (or not catch them in the first
place). :(
>
>> On 10 Jan 2017, at 08:32, Oleg Ranevskyy via llvm-dev <llvm-dev at
lists.llvm.org> wrote:
>>
>> 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
>> _______________________________________________
>> LLVM Developers mailing list
>> llvm-dev at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>
> -- Dean
>
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev


-- 
With best regards, Anton Korobeynikov
Department of Statistical Modelling, Saint Petersburg State University

> On 10 Jan 2017, at 16:41, Anton Korobeynikov <anton at
korobeynikov.info> wrote:
> 
> Why there are no tests that would break in this case?
> 
As Oleg points out, apparently the way we've configured the XRay tests in
compiler-rt/... excludes running tests on non-64-bit platforms. arm7 is
definitely not 64-bit and therefore means we've not been running those
tests. :(
> On Mon, Jan 9, 2017 at 8:32 PM, Dean Michael Berris via llvm-dev
> <llvm-dev at lists.llvm.org> wrote:
>> Oops -- sorry about that, this is definitely unintended. Adding in
Martin who made the change.
>> 
>> I'm happy to review changes to fix this, or if you don't get to
it first I can drive by that part of the code and try and fix it myself. I
unfortunately don't have easy access to arm machines so it is a little hard
for me to try and fix these things proactively (or not catch them in the first
place). :(
>> 
>>> On 10 Jan 2017, at 08:32, Oleg Ranevskyy via llvm-dev <llvm-dev
at lists.llvm.org> wrote:
>>> 
>>> 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.
>>> 
The change that will bring back the instrumentation map should contain the fix
to this configuration condition.

Serge, are you happy to do that as it seems you have better access to this
platform? I suspect Martin could also give it a go.

-- Dean

> As Oleg points out, apparently the way we've configured the XRay tests
in compiler-rt/... excludes running tests on non-64-bit platforms. arm7 is
definitely not 64-bit and therefore means we've not been running those
tests. :(
I believe that for this particular case one does not need to have
execution tests - you'd simply need to check for expected output and
that's it.

-- 
With best regards, Anton Korobeynikov
Department of Statistical Modelling, Saint Petersburg State University