vsk added a subscriber: vsk. vsk added a comment. Thanks, comments inline --
================ Comment at: lib/Driver/Tools.cpp:4064 @@ -4046,1 +4063,3 @@ + // Add runtime flag for PS4 when PGO or Coverage are enabled. + if (getToolChain().getTriple().isPS4CPU()) ---------------- Profiling instrumentation seems orthogonal to PIC. Could you keep the calls to `addPS4ProfileRTArgs` in their original locations? ================ Comment at: test/Driver/ps4-runtime-flags.c:8 @@ +7,3 @@ +// RUN: %clang -target x86_64-scei-ps4 -fprofile-arcs %s -### 2>&1 | FileCheck --check-prefix=CHECK-PS4-PROFILE %s +// RUN: %clang -target x86_64-scei-ps4 -fno-profile-arcs -fprofile-arcs %s -### 2>&1 | FileCheck --check-prefix=CHECK-PS4-PROFILE %s +// RUN: %clang -target x86_64-scei-ps4 -fno-profile-arcs %s -### 2>&1 | FileCheck -check-prefix=CHECK-PS4-NO-PROFILE %s ---------------- This tests whether ArgList::hasFlag() works. It doesn't really stress your patch. Please omit it. ================ Comment at: test/Driver/ps4-runtime-flags.c:10 @@ +9,3 @@ +// RUN: %clang -target x86_64-scei-ps4 -fno-profile-arcs %s -### 2>&1 | FileCheck -check-prefix=CHECK-PS4-NO-PROFILE %s +// RUN: %clang -target x86_64-scei-ps4 -fprofile-arcs -fno-profile-arcs %s -### 2>&1 | FileCheck --check-prefix=CHECK-PS4-NO-PROFILE %s +// RUN: %clang -target x86_64-scei-ps4 -fno-profile-arcs %s -### 2>&1 | FileCheck -check-prefix=CHECK-PS4-NO-PROFILE %s ---------------- ^ Ditto. ================ Comment at: test/Driver/ps4-runtime-flags.c:11 @@ +10,3 @@ +// RUN: %clang -target x86_64-scei-ps4 -fprofile-arcs -fno-profile-arcs %s -### 2>&1 | FileCheck --check-prefix=CHECK-PS4-NO-PROFILE %s +// RUN: %clang -target x86_64-scei-ps4 -fno-profile-arcs %s -### 2>&1 | FileCheck -check-prefix=CHECK-PS4-NO-PROFILE %s +// RUN: %clang -target x86_64-scei-ps4 -fprofile-generate %s -### 2>&1 | FileCheck --check-prefix=CHECK-PS4-PROFILE %s ---------------- Looks identical to line 9. Unless I'm missing something, this should be removed. ================ Comment at: test/Driver/ps4-runtime-flags.c:14 @@ +13,3 @@ +// RUN: %clang -target x86_64-scei-ps4 -fprofile-generate=dir %s -### 2>&1 | FileCheck --check-prefix=CHECK-PS4-PROFILE %s +// RUN: %clang -target x86_64-scei-ps4 -fno-profile-generate -fprofile-generate %s -### 2>&1 | FileCheck --check-prefix=CHECK-PS4-PROFILE %s +// RUN: %clang -target x86_64-scei-ps4 -fno-profile-generate %s -### 2>&1 | FileCheck --check-prefix=CHECK-PS4-NO-PROFILE %s ---------------- Omit? ================ Comment at: test/Driver/ps4-runtime-flags.c:16 @@ +15,3 @@ +// RUN: %clang -target x86_64-scei-ps4 -fno-profile-generate %s -### 2>&1 | FileCheck --check-prefix=CHECK-PS4-NO-PROFILE %s +// RUN: %clang -target x86_64-scei-ps4 -fprofile-generate -fno-profile-generate %s -### 2>&1 | FileCheck --check-prefix=CHECK-PS4-NO-PROFILE %s +// RUN: %clang -target x86_64-scei-ps4 -fprofile-generate -fno-profile-instr-generate %s -### 2>&1 | FileCheck --check-prefix=CHECK-PS4-NO-PROFILE %s ---------------- This one too. ================ Comment at: test/Driver/ps4-runtime-flags.c:22 @@ +21,3 @@ +// RUN: %clang -target x86_64-scei-ps4 -fprofile-instr-generate=somefile.profraw %s -### 2>&1 | FileCheck --check-prefix=CHECK-PS4-PROFILE %s +// RUN: %clang -target x86_64-scei-ps4 -fno-profile-instr-generate -fprofile-instr-generate %s -### 2>&1 | FileCheck --check-prefix=CHECK-PS4-PROFILE %s +// RUN: %clang -target x86_64-scei-ps4 -fprofile-instr-generate -fcoverage-mappinge %s -### 2>&1 | FileCheck --check-prefix=CHECK-PS4-PROFILE %s ---------------- And this. ================ Comment at: test/Driver/ps4-runtime-flags.c:23 @@ +22,3 @@ +// RUN: %clang -target x86_64-scei-ps4 -fno-profile-instr-generate -fprofile-instr-generate %s -### 2>&1 | FileCheck --check-prefix=CHECK-PS4-PROFILE %s +// RUN: %clang -target x86_64-scei-ps4 -fprofile-instr-generate -fcoverage-mappinge %s -### 2>&1 | FileCheck --check-prefix=CHECK-PS4-PROFILE %s +// RUN: %clang -target x86_64-scei-ps4 -fno-profile-instr-generate %s -### 2>&1 | FileCheck --check-prefix=CHECK-PS4-NO-PROFILE %s ---------------- Typo: `-fcoverage-mappinge` ================ Comment at: test/Driver/ps4-runtime-flags.c:24 @@ +23,3 @@ +// RUN: %clang -target x86_64-scei-ps4 -fprofile-instr-generate -fcoverage-mappinge %s -### 2>&1 | FileCheck --check-prefix=CHECK-PS4-PROFILE %s +// RUN: %clang -target x86_64-scei-ps4 -fno-profile-instr-generate %s -### 2>&1 | FileCheck --check-prefix=CHECK-PS4-NO-PROFILE %s +// RUN: %clang -target x86_64-scei-ps4 -fprofile-instr-generate -fno-profile-instr-generate %s -### 2>&1 | FileCheck --check-prefix=CHECK-PS4-NO-PROFILE %s ---------------- The rest of these feel a bit redundant, but I haven't looked too closely. http://reviews.llvm.org/D15222 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
