Removed a call to EmitXRayTable() from ARMAsmPrinter

Sharing with the mailing list… Please, see below.

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

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). :frowning:

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

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. :frowning:

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. :frowning:

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.

Oh, that's a good point. Yes, we definitely ought to be looking for the instrumentation map in the codegen tests for arm7.

That's probably something we can set up in the same change, and ought to be simple to do anyway.

-- Dean

Then I plan to enable back emission of XRay instrumentation map in Arm32 and testing of Arm32 (about 1 line each, but they may break the builds), and to submit this for review together with the tail call patches.

Serge

Thanks Serge, looking forward to reviewing that patch. :slight_smile:

Cheers

-- Dean