Re: [PATCH] D15222: [Patch][Profile] add “--dependent-lib= libclang_rt.profile-x86_64.a” to the CC1 command line when enabling code coverage on PS4

Vedant Kumar via cfe-commits cfe-commits at lists.llvm.org
Mon Dec 14 08:40:25 PST 2015


vsk added inline comments.

================
Comment at: lib/Driver/Tools.cpp:4067
@@ -4049,1 +4066,3 @@
 
+  // Add runtime flag for PS4 when PGO or Coverage are enabled.
+  if (getToolChain().getTriple().isPS4CPU())
----------------
Sorry, I don't know why I thought this was in ParsePIC. This seems fine.

================
Comment at: test/Driver/ps4-runtime-flags.c:9
@@ +8,3 @@
+// 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
+// RUN: %clang -target x86_64-scei-ps4 -fprofile-arcs -fno-profile-arcs %s -### 2>&1 | FileCheck --check-prefix=CHECK-PS4-NO-PROFILE %s
----------------
Got it, I see that there's a behavior change. My point was that it isn't necessary to test every combination in {-fX, -fY, -fno-X, -fno-Y, ...} x {-fX, -fY, -fno-X, -fno-Y, ...} to be sure the code works. Most users of hasFlag() aren't tested in this way since it'd cause a test case explosion.

I think lines 7-10 are all that's really needed to test the change from hasArg to hasFlag in your patch. What do you think?


http://reviews.llvm.org/D15222





More information about the cfe-commits mailing list