<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Wed, Oct 14, 2015 at 12:54 AM, Romanova, Katya <span dir="ltr"><<a href="mailto:Katya_Romanova@playstation.sony.com" target="_blank">Katya_Romanova@playstation.sony.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<div lang="EN-US" link="blue" vlink="purple">
<div>
<p class="MsoNormal"><span style="font-size:11pt;font-family:Calibri,sans-serif;color:rgb(31,73,125)">Hi Sean,<u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-size:11pt;font-family:Calibri,sans-serif;color:rgb(31,73,125)"><u></u> <u></u></span></p>
<p class="MsoNormal"><span style="font-size:11pt;font-family:Calibri,sans-serif;color:rgb(31,73,125)">I don’t think that the whole patch should have been reverted. There were much easier way to make the PS4 bot green. A single test could have been marked as
“XFAIL: ps4” for a few hours until a better solution is implemented. Reverting this huge patch is more drastic measure that might cause more problems later.<u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-size:11pt;font-family:Calibri,sans-serif;color:rgb(31,73,125)"><u></u> <u></u></span></p>
<p class="MsoNormal"><span style="font-size:11pt;font-family:Calibri,sans-serif;color:rgb(31,73,125)">As you saw in my previous email, I kept an eye on the bots, noticed the failure on PS4 buildbot, explained why it happened in the email and ask community for
an opinion if it could be temporarily marked as “XFAIL” for ps4 only. I had to leave work at certain point. When I got home, I saw that you reverted the whole fix. Now I will have to go through all the pain with maintaining a huge commit, instead of making
one line change affecting one of the tests on PS4 platform only… But, I guess, if it’s a general practice, then it’s a different story. I was not aware of this practice and I thought that the other solution was much more reasonable.</span></p></div></div></blockquote><div><br></div><div>Yes, it is general practice. Reverting is easier and sends a clear signal that broken code is not allowed. XFAIL gives the impression that it is failing for some reason that is not preventable (for example, the operating system doesn't support some system call that is needed); that is not true in this case: there was just an oversight and broken code was committed.</div><div><br></div><div>Also, the bot had been red for a *very* long time and there was no indication that you were doing anything about it (I reverted at almost midnight). In the future, be sure to keep an eye on the bots and either commit a fix or revert to green. "commit and go home" is very inconsiderate to all fellow community members.</div><div><br></div><div>-- Sean Silva</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div lang="EN-US" link="blue" vlink="purple"><div><p class="MsoNormal"><span style="font-size:11pt;font-family:Calibri,sans-serif;color:rgb(31,73,125)"><u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-size:11pt;font-family:Calibri,sans-serif;color:rgb(31,73,125)"><u></u> <u></u></span></p>
<p class="MsoNormal"><span style="font-size:11pt;font-family:Calibri,sans-serif;color:rgb(31,73,125)">Thank you for letting me know about
</span><span style="font-size:11pt;font-family:Calibri,sans-serif;color:rgb(31,73,125)">LLVM_DEFAULT_TARGET_TRIPLE=x86_64-scei-ps4. I will definitely run it before the next commit.</span><span style="font-size:11pt;font-family:Calibri,sans-serif;color:rgb(31,73,125)"><u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-size:11pt;font-family:Calibri,sans-serif;color:rgb(31,73,125)"><u></u> <u></u></span></p>
<p class="MsoNormal"><span style="font-size:11pt;font-family:Calibri,sans-serif;color:rgb(31,73,125)">Katya.<u></u><u></u></span></p>
<p class="MsoNormal"><span style="font-size:11pt;font-family:Calibri,sans-serif;color:rgb(31,73,125)"><u></u> <u></u></span></p>
<p class="MsoNormal"><a name="15068f000d9ec4d5_1506558498e67a63__MailEndCompose"><span style="font-size:11pt;font-family:Calibri,sans-serif;color:rgb(31,73,125)"><u></u> <u></u></span></a></p>
<div style="border-style:none none none solid;border-left-color:blue;border-left-width:1.5pt;padding:0in 0in 0in 4pt">
<div>
<div style="border-style:solid none none;border-top-color:rgb(181,196,223);border-top-width:1pt;padding:3pt 0in 0in">
<p class="MsoNormal"><b><span style="font-size:10pt;font-family:Tahoma,sans-serif">From:</span></b><span style="font-size:10pt;font-family:Tahoma,sans-serif"> Sean Silva [mailto:<a href="mailto:chisophugis@gmail.com" target="_blank">chisophugis@gmail.com</a>]
<br>
<b>Sent:</b> Tuesday, October 13, 2015 11:56 PM<br>
<b>To:</b> Eric Christopher<br>
<b>Cc:</b> <a href="mailto:reviews%2BD13482%2Bpublic%2Ba1a9627af638cb2e@reviews.llvm.org" target="_blank">reviews+D13482+public+a1a9627af638cb2e@reviews.llvm.org</a>; Romanova, Katya; Alex Rosenberg; Robinson, Paul; <a href="mailto:filcab%2Bllvm.phabricator@gmail.com" target="_blank">filcab+llvm.phabricator@gmail.com</a>; Jonathan Roelofs; Bedwell, Greg; <a href="mailto:pierregousseau14@gmail.com" target="_blank">pierregousseau14@gmail.com</a>; Anton Korobeynikov; Takumi NAKAMURA; cfe-commits<br>
<b>Subject:</b> Re: [PATCH] D13482: Revised Initial patch for PS4 toolchain<u></u><u></u></span></p>
</div>
</div><div><div>
<p class="MsoNormal"><u></u> <u></u></p>
<div>
<p class="MsoNormal"><u></u> <u></u></p>
<div>
<p class="MsoNormal"><u></u> <u></u></p>
<div>
<p class="MsoNormal">On Tue, Oct 13, 2015 at 11:40 PM, Eric Christopher <<a href="mailto:echristo@gmail.com" target="_blank">echristo@gmail.com</a>> wrote:<u></u><u></u></p>
<div>
<p class="MsoNormal" style="margin-bottom:12pt"><u></u> <u></u></p>
<div>
<div>
<p class="MsoNormal">On Tue, Oct 13, 2015 at 11:38 PM Sean Silva <<a href="mailto:chisophugis@gmail.com" target="_blank">chisophugis@gmail.com</a>> wrote:<u></u><u></u></p>
</div>
<blockquote style="border-style:none none none solid;border-left-color:rgb(204,204,204);border-left-width:1pt;padding:0in 0in 0in 6pt;margin-left:4.8pt;margin-right:0in">
<div>
<div>
<div>
<p class="MsoNormal">On Tue, Oct 13, 2015 at 11:33 PM, Eric Christopher <<a href="mailto:echristo@gmail.com" target="_blank">echristo@gmail.com</a>> wrote:<u></u><u></u></p>
<div>
<div>
<p class="MsoNormal">It was already reverted, but I agree, let's get this fixed first.<u></u><u></u></p>
</div>
</div>
<div>
<p class="MsoNormal"><u></u> <u></u></p>
</div>
</div>
</div>
</div>
<div>
<div>
<div>
<div>
<p class="MsoNormal">It was reintroduced in r250252. It is breaking <a href="http://lab.llvm.org:8011/builders/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/builds/1362" target="_blank">http://lab.llvm.org:8011/builders/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/builds/1362</a><u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"><u></u> <u></u></p>
</div>
</div>
</div>
</div>
</blockquote>
<div>
<p class="MsoNormal"><u></u> <u></u></p>
</div>
<div>
<p class="MsoNormal">Ah, I missed that. Yeah, please go ahead and revert for now.<u></u><u></u></p>
</div>
</div>
</div>
<div>
<p class="MsoNormal"><u></u> <u></u></p>
</div>
<div>
<p class="MsoNormal">reverted in r250273<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal">(bot is back to green: <a href="http://lab.llvm.org:8011/builders/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/builds/1363" target="_blank">http://lab.llvm.org:8011/builders/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/builds/1363</a> )<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"><u></u> <u></u></p>
</div>
<div>
<p class="MsoNormal">Katya - remember to run the tests with LLVM_DEFAULT_TARGET_TRIPLE=x86_64-scei-ps4 (and also remember to keep an eye on the bots after committing)<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"><u></u> <u></u></p>
</div>
<div>
<p class="MsoNormal">-- Sean Silva<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"> <u></u><u></u></p>
</div>
<blockquote style="border-style:none none none solid;border-left-color:rgb(204,204,204);border-left-width:1pt;padding:0in 0in 0in 6pt;margin-left:4.8pt;margin-right:0in">
<div>
<div>
<div>
<p class="MsoNormal"><u></u> <u></u></p>
</div>
<div>
<p class="MsoNormal">Thanks!<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"><span style="color:rgb(136,136,136)"><u></u> <u></u></span></p>
</div>
<div>
<p class="MsoNormal"><span style="color:rgb(136,136,136)">-eric<u></u><u></u></span></p>
</div>
<div>
<div>
<div>
<p class="MsoNormal"> <u></u><u></u></p>
</div>
<blockquote style="border-style:none none none solid;border-left-color:rgb(204,204,204);border-left-width:1pt;padding:0in 0in 0in 6pt;margin-left:4.8pt;margin-right:0in">
<div>
<div>
<div>
<div>
<p class="MsoNormal">-- Sean Silva<u></u><u></u></p>
</div>
</div>
</div>
</div>
<div>
<div>
<div>
<div>
<p class="MsoNormal"> <u></u><u></u></p>
</div>
<blockquote style="border-style:none none none solid;border-left-color:rgb(204,204,204);border-left-width:1pt;padding:0in 0in 0in 6pt;margin-left:4.8pt;margin-right:0in">
<div>
<div>
<p class="MsoNormal"><u></u> <u></u></p>
</div>
<div>
<p class="MsoNormal">Thanks!<u></u><u></u></p>
</div>
<div>
<p class="MsoNormal"><span style="color:rgb(136,136,136)"><u></u> <u></u></span></p>
</div>
<div>
<p class="MsoNormal"><span style="color:rgb(136,136,136)">-eric<u></u><u></u></span></p>
</div>
</div>
<div>
<div>
<p class="MsoNormal"><u></u> <u></u></p>
<div>
<div>
<p class="MsoNormal">On Tue, Oct 13, 2015 at 11:33 PM Sean Silva <<a href="mailto:chisophugis@gmail.com" target="_blank">chisophugis@gmail.com</a>> wrote:<u></u><u></u></p>
</div>
<blockquote style="border-style:none none none solid;border-left-color:rgb(204,204,204);border-left-width:1pt;padding:0in 0in 0in 6pt;margin-left:4.8pt;margin-right:0in">
<div>
<div>
<div>
<p class="MsoNormal">On Tue, Oct 13, 2015 at 7:51 PM, Katya Romanova <<a href="mailto:Katya_Romanova@playstation.sony.com" target="_blank">Katya_Romanova@playstation.sony.com</a>> wrote:<u></u><u></u></p>
<p class="MsoNormal">kromanova added a comment.<br>
<br>
Hi,<br>
<br>
The initial PS4 patch caused a test failure (debug-options.c) on the PS4 bot. I suspect that I know why the problem happens, but I'm not sure what will be the best way to handle it. If someone knows how to fix this test more "elegantly", I would appreciate
their advice.<br>
<br>
FAIL: Clang :: Driver/debug-options.c (3509 of 24708)<br>
<br>
- TEST 'Clang :: Driver/debug-options.c' FAILED ********************<br>
<br>
Script:<br>
-------<br>
<br>
/home/buildbot/Buildbot/Slave/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/./bin/clang -### -c -g /home/buildbot/Buildbot/Slave/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.src/tools/clang/test/Driver/debug-options.c -target x86_64-linux-gnu 2>&1
| /home/buildbot/Buildbot/Slave/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/./bin/FileCheck -check-prefix=G /home/buildbot/Buildbot/Slave/llvm-clang-lld-x86_64-scei-<br>
....<br>
<br>
/home/buildbot/Buildbot/Slave/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/./bin/clang -### -g /home/buildbot/Buildbot/Slave/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.src/tools/clang/test/Driver/debug-options.c 2>&1 | /home/buildbot/Buildbot/Slave/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/./bin/FileCheck
-check-prefix=CI /home/buildbot/Buildbot/Slave/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.src/tools/clang/test/Driver/debug-options.c<br>
------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------<br>
<br>
Exit Code: 1<br>
<br>
Command Output (stderr):<br>
------------------------<br>
<br>
/home/buildbot/Buildbot/Slave/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.src/tools/clang/test/Driver/debug-options.c:139:8: error: expected string not found in input<br>
// CI: "-dwarf-column-info"<br>
<br>
^<br>
<br>
<stdin>:1:1: note: scanning from here<br>
clang version 3.8.0 (trunk 250262)<br>
^<br>
<stdin>:5:438: note: possible intended match here<br>
"/home/buildbot/Buildbot/Slave/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/bin/clang-3.8" "-cc1" "-triple" "x86_64-scei-ps4" "-emit-obj" "-mrelax-all" "-disable-free" "-main-file-name" "debug-options.c" "-mrelocation-model" "pic" "-pic-level" "2" "-mthread-model"
"posix" "-mdisable-fp-elim" "-masm-verbose" "-mconstructor-aliases" "-munwind-tables" "-target-cpu" "btver2" "-momit-leaf-frame-pointer" "-debug-info-kind=limited" "-dwarf-version=4" "-backend-option" "-generate-arange-section" "-resource-dir" "/home/buildbot/Buildbot/Slave/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/bin/../lib/clang/3.8.0"
"-fdebug-compilation-dir" "/home/buildbot/Buildbot/Slave/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/tools/clang/test/Driver" "-ferror-limit" "19" "-fmessage-length" "0" "-stack-protector" "2" "-fdeclspec" "-fobjc-runtime=gnustep" "-fdiagnostics-show-option"
"-o" "/tmp/debug-options-1505f5.o" "-x" "c" "/home/buildbot/Buildbot/Slave/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.src/tools/clang/test/Driver/debug-options.c"<br>
<br>
^<br>
<br>
-<br>
<br>
The latest driver patch introduced a change, causing the PS4 driver *not* to have -gcolumn-info enabled by default.<br>
<br>
Consequently, this generic line started to fail on the PS4 bot.<br>
// RUN: %clang -### -g %s 2>&1 | FileCheck -check-prefix=CI %s<br>
<br>
Does someone know what will be the best way to run the test line for all the platforms, except PS4?<br>
<br>
In the patch, we have added a couple of PS4 specific lines to this test, to verify that the behavior on PS4 is correct:<br>
<br>
// RUN: %clang -### -c %s -g -target x86_64-scei-ps4 2>&1 \<br>
// RUN: | FileCheck -check-prefix=NOCI %s<br>
<br>
// RUN: %clang -### -c %s -g -gcolumn-info -target x86_64-scei-ps4 2>&1 \<br>
// RUN: | FileCheck -check-prefix=CI %s<br>
<br>
I do not want to make this test XFAIL for PS4 (though I might do it as an interim solution). I would also prefer to avoid duplicating most of the content of this file into a separate ps4-specific file.<br>
Any ideas how to handle this issue "more elegantly"?<br>
<br>
If nobody objects, I will mark this test as XFAIL for PS4 for a time being.<u></u><u></u></p>
<div>
<p class="MsoNormal"><u></u> <u></u></p>
</div>
</div>
</div>
</div>
<div>
<div>
<div>
<div>
<p class="MsoNormal">Please revert until you can solve the issue.<u></u><u></u></p>
</div>
</div>
</div>
</div>
<div>
<div>
<div>
<div>
<p class="MsoNormal"><u></u> <u></u></p>
</div>
<div>
<p class="MsoNormal">-- Sean Silva<u></u><u></u></p>
</div>
</div>
</div>
</div>
<div>
<div>
<div>
<div>
<p class="MsoNormal"> <u></u><u></u></p>
</div>
<blockquote style="border-style:none none none solid;border-left-color:rgb(204,204,204);border-left-width:1pt;padding:0in 0in 0in 6pt;margin-left:4.8pt;margin-right:0in">
<p class="MsoNormal"><span style="color:rgb(136,136,136)"><br>
Katya.</span><u></u><u></u></p>
<div>
<div>
<p class="MsoNormal" style="margin-bottom:12pt"><br>
<br>
Repository:<br>
rL LLVM<br>
<br>
<a href="http://reviews.llvm.org/D13482" target="_blank">http://reviews.llvm.org/D13482</a><br>
<br>
<br>
<u></u><u></u></p>
</div>
</div>
</blockquote>
</div>
</div>
</div>
</blockquote>
</div>
</div>
</div>
</blockquote>
</div>
</div>
</div>
</blockquote>
</div>
</div>
</div>
</div>
</blockquote>
</div>
<p class="MsoNormal"><u></u> <u></u></p>
</div>
</div>
</div></div></div>
</div>
</div>
</blockquote></div><br></div></div>