Oleg Ranevskyy via llvm-dev
2017-Jan-09 21:32 UTC
[llvm-dev] Removed a call to EmitXRayTable() from ARMAsmPrinter
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
Dean Michael Berris via llvm-dev
2017-Jan-10 04:32 UTC
[llvm-dev] Removed a call to EmitXRayTable() from ARMAsmPrinter
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
Anton Korobeynikov via llvm-dev
2017-Jan-10 05:41 UTC
[llvm-dev] 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