Filipe,
I have one question regarding the change in debug-option.c that you made to get 
the builds green again… (r250294).

Basically you changed the generic run line that used to test the drivers for 
every single platform
// RUN: %clang -### -g %s 2>&1 | FileCheck -check-prefix=CI %s

into the run line that tests x86_64 drivers only.
// RUN: %clang -### -g -target x86_64-unknown-unknown %s 2>&1 \
//             | FileCheck -check-prefix=CI %s

By making this change, we reduced driver coverage for the other platforms, e.g. 
AMD.  This run line will continue to pass for AMD irrespective of whether the 
AMD driver enabled or disabled emitting column information by default. I 
suspect that *non*-x86_64 developers won't be happy that we reduced their 
drivers’ test coverage without letting them know.

Is there a better way to address this issue?

From what I understand, it’s impossible now to execute a specific RUN line for 
all the platforms, except platform <foo>. However, it’s easy to do it for the 
whole file (XFAIL: <foo>). Does anyone know how hard it would be to expand lit 
functionality to do XFAIL: <foo> for a block of RUN lines in a file (e.g. XFAIL 
: begin: <foo> …. XFAIL : end : <foo>? Or, even a simpler approach: force XFAIL 
to take effect only for the RUN lines that follow XFAIL line. It will 
definitely add much more power/flexibility to FAIL functionality in lit. It 
might also reduce test code duplication. What do you think?

But for now (for our specific problem)  maybe it’s better to remove the run 
lines that work differently for PS4 and for the rest of the world from 
debug_options.c  file and place them into a ps4 specific file (e.g. 
ps4-Gcolumn-info.c) and into non-ps4 generic file (e.g gcolumn-info.c) and make 
gcolumn-info.c to XFAIL for PS4. We could do this extraction for gcolumn_info, 
debug_aranges, and anything else that is different for PS4 and everyone else,

Katya.


From: fil...@gmail.com [mailto:fil...@gmail.com] On Behalf Of Filipe Cabecinhas
Sent: Wednesday, October 14, 2015 6:11 AM
To: Romanova, Katya
Cc: Sean Silva; Eric Christopher; 
reviews+d13482+public+a1a9627af638c...@reviews.llvm.org; Alex Rosenberg; 
Robinson, Paul; Jonathan Roelofs; Bedwell, Greg; pierregoussea...@gmail.com; 
Anton Korobeynikov; Takumi NAKAMURA; cfe-commits
Subject: Re: [PATCH] D13482: Revised Initial patch for PS4 toolchain

Hi all,

I have harnessed the powers of time zones and did the last tweak we needed to 
get it comitted.
It's still going through some bots, but I don't foresee big problems. The fast 
bots liked it.

Thanks to everyone that reviewed it and worked on it!

  Filipe

On Wed, Oct 14, 2015 at 8:54 AM, Romanova, Katya 
<katya_roman...@playstation.sony.com<mailto:katya_roman...@playstation.sony.com>>
 wrote:
Hi Sean,

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.

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.

Thank you for letting me know about LLVM_DEFAULT_TARGET_TRIPLE=x86_64-scei-ps4. 
I will definitely run it before the next commit.

Katya.


From: Sean Silva [mailto:chisophu...@gmail.com<mailto:chisophu...@gmail.com>]
Sent: Tuesday, October 13, 2015 11:56 PM
To: Eric Christopher
Cc: 
reviews+d13482+public+a1a9627af638c...@reviews.llvm.org<mailto:reviews%2bd13482%2bpublic%2ba1a9627af638c...@reviews.llvm.org>;
 Romanova, Katya; Alex Rosenberg; Robinson, Paul; 
filcab+llvm.phabrica...@gmail.com<mailto:filcab%2bllvm.phabrica...@gmail.com>; 
Jonathan Roelofs; Bedwell, Greg; 
pierregoussea...@gmail.com<mailto:pierregoussea...@gmail.com>; Anton 
Korobeynikov; Takumi NAKAMURA; cfe-commits
Subject: Re: [PATCH] D13482: Revised Initial patch for PS4 toolchain



On Tue, Oct 13, 2015 at 11:40 PM, Eric Christopher 
<echri...@gmail.com<mailto:echri...@gmail.com>> wrote:

On Tue, Oct 13, 2015 at 11:38 PM Sean Silva 
<chisophu...@gmail.com<mailto:chisophu...@gmail.com>> wrote:
On Tue, Oct 13, 2015 at 11:33 PM, Eric Christopher 
<echri...@gmail.com<mailto:echri...@gmail.com>> wrote:
It was already reverted, but I agree, let's get this fixed first.

It was reintroduced in r250252. It is breaking 
http://lab.llvm.org:8011/builders/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/builds/1362


Ah, I missed that. Yeah, please go ahead and revert for now.

reverted in r250273
(bot is back to green: 
http://lab.llvm.org:8011/builders/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/builds/1363
 )

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)

-- Sean Silva


Thanks!

-eric

-- Sean Silva


Thanks!

-eric

On Tue, Oct 13, 2015 at 11:33 PM Sean Silva 
<chisophu...@gmail.com<mailto:chisophu...@gmail.com>> wrote:
On Tue, Oct 13, 2015 at 7:51 PM, Katya Romanova 
<katya_roman...@playstation.sony.com<mailto:katya_roman...@playstation.sony.com>>
 wrote:
kromanova added a comment.

Hi,

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.

FAIL: Clang :: Driver/debug-options.c (3509 of 24708)

- TEST 'Clang :: Driver/debug-options.c' FAILED ********************

Script:
-------

/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-
....

/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
------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------

Exit Code: 1

Command Output (stderr):
------------------------

/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
// CI: "-dwarf-column-info"

  ^

<stdin>:1:1: note: scanning from here
clang version 3.8.0 (trunk 250262)
^
<stdin>:5:438: note: possible intended match here
 
"/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"

                                                                                
                                                                                
                                                                                
                                                                                
                                       ^

-

The latest driver patch introduced a change, causing the PS4 driver *not* to 
have -gcolumn-info enabled by default.

Consequently, this generic line started to fail on the PS4 bot.
// RUN: %clang -### -g %s 2>&1 | FileCheck -check-prefix=CI %s

Does someone know what will be the best way to run the test line for all the 
platforms, except PS4?

In the patch, we have added a couple of PS4 specific lines to this test, to 
verify that the behavior on PS4 is correct:

// RUN: %clang -### -c %s -g -target x86_64-scei-ps4 2>&1 \
// RUN:             | FileCheck -check-prefix=NOCI %s

// RUN: %clang -### -c %s -g -gcolumn-info -target x86_64-scei-ps4 2>&1 \
// RUN:             | FileCheck -check-prefix=CI %s

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.
Any ideas how to handle this issue "more elegantly"?

If nobody objects, I will mark this test as XFAIL for PS4 for a time being.

Please revert until you can solve the issue.

-- Sean Silva


Katya.


Repository:
  rL LLVM

http://reviews.llvm.org/D13482



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to