[llvm-dev] Critical XRay fixes for Arm32

[Hans Wennborg via llvm-dev]

I see. Thanks for clarifying.

I'm Ok with merging these if Dean agrees, as I believe he's the code owner.

Thanks,
Hans

On Thu, Jan 26, 2017 at 11:47 AM, Serge Rogatch <serge.rogatch at gmail.com> wrote:
> There were no LLVM tests for presence of XRay instrumentation map in the
> emitted assembly. You can see that https://reviews.llvm.org/D28624 adds this
> check to the tests.
> The tests in compiler-rt had been accidentally disabled.
> https://reviews.llvm.org/D28623 enables them in
> compiler-rt/test/xray/lit.cfg .
>
> On 26 January 2017 at 22:37, Hans Wennborg <hans at chromium.org> wrote:
>>
>> I'm wondering why the lit tests didn't catch this as part of testing rc1
>> on ARM.
>>
>> On Thu, Jan 26, 2017 at 11:25 AM, Serge Rogatch <serge.rogatch at gmail.com>
>> wrote:
>> > XRay is tested automatically on build-bots with tests in LLVM and
>> > compiler-rt . Or are you asking for manual testing instructions?
>> > Of these 2 patches, the compiler-rt patch depends on LLVM patch because
>> > the
>> > tests compiler-rt\test\xray\TestCases\Linux would fail without the fix
>> > in
>> > LLVM. The compiler-rt patch also enables the tests which were
>> > occasionally
>> > disabled.
>> >
>> > The XRay feature is quite isolated, so side-effects are not expected. I
>> > can
>> > only see a risk that some commit I took for granted when making&testing
>> > the
>> > fixes, somehow didn't get to 4.0 branch.
>> >
>> >
>> > On 26 January 2017 at 21:44, Hans Wennborg <hans at chromium.org> wrote:
>> >>
>> >> How is XRay tested? IIRC, Renato didn't see any test failures on ARM?
>> >>
>> >> Merging sounds reasonbaly, I'd just like to understand what's the risk
>> >> for the branch.
>> >>
>> >> On Thu, Jan 26, 2017 at 10:29 AM, Serge Rogatch
>> >> <serge.rogatch at gmail.com>
>> >> wrote:
>> >> > Hans, these changes reached trunk in
>> >> > https://reviews.llvm.org/rL292516
>> >> > and
>> >> > https://reviews.llvm.org/rL292517 . Could you look?
>> >> >
>> >> > On 26 January 2017 at 03:29, Serge Rogatch <serge.rogatch at gmail.com>
>> >> > wrote:
>> >> >>
>> >> >> Sorry, I initially included LLVM-Commits rather than LLVM-Dev.
>> >> >> Fixed.
>> >> >>
>> >> >> On 26 January 2017 at 03:26, Serge Rogatch <serge.rogatch at gmail.com>
>> >> >> wrote:
>> >> >>>
>> >> >>> Hi Dean, Renato,
>> >> >>>
>> >> >>> AFAIK, unfortunately, these critical Arm32 XRay fixes are not yet
>> >> >>> in
>> >> >>> 4.0:
>> >> >>> https://reviews.llvm.org/D28624 , https://reviews.llvm.org/D28623 .
>> >> >>> The
>> >> >>> first repairs XRay instrumentation map emission. Without this map
>> >> >>> XRay
>> >> >>> doesn't work at all: the runtime doesn't see anything to
>> >> >>> instrument.
>> >> >>> The
>> >> >>> second fixes the CPU cache incoherency problem. Without this patch,
>> >> >>> XRay
>> >> >>> will intermittently fail to patch or unpatch some sleds (depending
>> >> >>> on
>> >> >>> whether their code is in CPU cache or not).
>> >> >>>
>> >> >>> Is there any chance we can get these patches to 4.0? This page
>> >> >>> http://llvm.org/docs/HowToReleaseLLVM.html#release-patch-rules says
>> >> >>> that
>> >> >>> "release manager, the official release testers or the code owners"
>> >> >>> may
>> >> >>> approve cherry-picking to release branch from trunk. AFAIK, you are
>> >> >>> code
>> >> >>> owners for XRay and ARM. I don't know who are the release manager
>> >> >>> or
>> >> >>> official release testers.
>> >> >>>
>> >> >>> Cheers,
>> >> >>> Serge
>> >> >>
>> >> >>
>> >> >
>> >
>> >
>
>