Page MenuHomePhabricator

[XRay][Arm] Enable back XRay testing on Arm32 and fix the failing tests
ClosedPublic

Authored by rSerge on Jan 12 2017, 12:31 PM.

Diff Detail

Event Timeline

rSerge updated this revision to Diff 84163.Jan 12 2017, 12:31 PM
rSerge retitled this revision from to [XRay][Arm] Enable back XRay testing on Arm32.
rSerge updated this object.
rSerge added a subscriber: iid_iunknown.
rSerge updated this object.Jan 12 2017, 12:35 PM
rSerge added reviewers: dberris, rengolin.
rSerge added a subscriber: llvm-commits.
dberris edited edge metadata.Jan 12 2017, 3:42 PM

Yes, that was me assuming that XRay wouldn't be supported in 32-bit systems. But I'm happy to be proven wrong there. :D

test/xray/lit.cfg
35

I'm not too familiar with the arm triples, but don't we only support xray on arm7? Does that distinction matter?

rSerge added inline comments.Jan 13 2017, 3:23 AM
test/xray/lit.cfg
35

AFAIK, build-bots are currently only armv7 . Furthermore, I understood that tests can only execute on architectures in ALL_XRAY_SUPPORTED_ARCH from llvm\projects\compiler-rt\cmake\config-ix.cmake , and this makes me question why 64-bitness is checked here again...

rengolin added inline comments.Jan 13 2017, 3:28 AM
test/xray/lit.cfg
35

FWIW, all bots are ARMv7 and ARMv8. But that can break if someone is running the tests on, for example, a Raspberry Pi 1.

rengolin edited edge metadata.Jan 13 2017, 3:30 AM

Yes, that was me assuming that XRay wouldn't be supported in 32-bit systems. But I'm happy to be proven wrong there. :D

Well, before we get this in, it would be good to make sure they actually pass all the tests on an ARMv7 system. I can run it locally on a system like the buildbots to be sure.

@rengolin , yes, please. This patch should be tested together with https://reviews.llvm.org/D28624 , because this patch only enables the tests and that one repairs XRay on Arm32.

test/xray/lit.cfg
35

Oh, I meant "at least armv7". Armv8 should be ok. Armv6 is not fully supported by XRay (because movw and movt are written to the code at runtime).

rSerge added inline comments.Jan 13 2017, 3:35 AM
test/xray/lit.cfg
35

Actually, I'm not sure about armv6, may be it's ok because movw and movt appear in armv6t2 .

rengolin added inline comments.Jan 13 2017, 3:53 AM
test/xray/lit.cfg
35

Raspberry Pi 1 should be fine, then.

rengolin requested changes to this revision.Jan 13 2017, 6:26 AM
rengolin edited edge metadata.

I just tested on ARM and got these errors:

TestCases/Linux/patching-unpatching.cc

patching-unpatching.cc:28:17: error: expected string not found in input
// CHECK-NEXT: patching status: 1
               ^
<stdin>:3:1: note: scanning from here
patching status: 0
^

TestCases/Linux/argv0-log-file-name.cc

xray.log.file.name:2:11: error: expected string not found in input
// CHECK: xray-log.argv0-log-file-name.cc.tmp.{{.*}}
          ^
<stdin>:1:1: note: scanning from here
Output
^
<stdin>:2:1: note: possible intended match here
xray.log.file.name
^

TestCases/Linux/fixedsize-logging.cc

fixedsize-logging.cc:17:12: error: expected string not found in input
// CHECK: XRay: Log file in 'fixedsize-logging-{{.*}}'
          ^
<stdin>:1:1: note: scanning from here
==27375==XRay instrumentation map missing. Not initializing XRay.
^
<stdin>:1:38: note: possible intended match here
==27375==XRay instrumentation map missing. Not initializing XRay.
                                     ^
This revision now requires changes to proceed.Jan 13 2017, 6:26 AM

@rengolin , did you first apply patch https://reviews.llvm.org/D28624 before testing the current patch?
Especially the last quoted failed test seems to behave as if emission of XRay table is still broken.

@rengolin , did you first apply patch https://reviews.llvm.org/D28624 before testing the current patch?
Especially the last quoted failed test seems to behave as if emission of XRay table is still broken.

Dang, no. I'll do it.

Almost. After applying both patches, I still get:

TestCases/Linux/patching-unpatching.cc

patching-unpatching.cc:30:17: error: expected string not found in input
 // CHECK-NEXT: called: {{.*}}, type=0
                ^
<stdin>:3:1: note: scanning from here
always instrumented called
^
<stdin>:3:21: note: possible intended match here
always instrumented called
                    ^

Almost. After applying both patches, I still get:

TestCases/Linux/patching-unpatching.cc

patching-unpatching.cc:30:17: error: expected string not found in input
 // CHECK-NEXT: called: {{.*}}, type=0
                ^
<stdin>:3:1: note: scanning from here
always instrumented called
^
<stdin>:3:21: note: possible intended match here
always instrumented called
                    ^

Likely it is the same problem with ARM instruction&data cache incoherency we've seen on AArch64. Let me replicate that fix here too...

Likely it is the same problem with ARM instruction&data cache incoherency we've seen on AArch64. Let me replicate that fix here too...

Makes sense.

rSerge updated this revision to Diff 84574.Jan 16 2017, 9:44 AM
rSerge retitled this revision from [XRay][Arm] Enable back XRay testing on Arm32 to [XRay][Arm] Enable back XRay testing on Arm32 and fix the failing tests.
rSerge updated this object.
rSerge edited edge metadata.

PTAL.

rengolin accepted this revision.Jan 16 2017, 12:17 PM
rengolin edited edge metadata.

Seems to work on my end. Thanks!

This revision is now accepted and ready to land.Jan 16 2017, 12:17 PM
rSerge closed this revision.Jan 17 2017, 4:04 AM

Reached mainline in 292211. Thanks all!

rSerge reopened this revision.Jan 19 2017, 11:41 AM
This revision is now accepted and ready to land.Jan 19 2017, 11:41 AM
rSerge updated this revision to Diff 84998.Jan 19 2017, 11:45 AM

Added detection of -mthumb test compiler option, so to mark tests as unsupported on Thumb because XRay doesn't yet support Thumb CPU mode on Arm32.

Makes sense. LGTM. Let's try again. :)

rSerge closed this revision.Jan 19 2017, 12:38 PM

With Thumb fix, in mainline at 292517.