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