[Clang][AIX][p] Manually claim -p in front end

[francii]

Summary

The current implementation of -p does not claim the argument once it is passed. Since it pushes -pg directly, it is only ever referred to again when linking. As a result, when compiling with -S, the compiler warns that -p goes unused even though that is not the case.

With this patch, if both -p and -pg are passed, the argument that is passed second will take precedence. -p will still throw an error on unsupported platforms, regardless of precedence.

This revision includes a test case, which has been placed in clang/test/Driver/zos-profiling-error.c. As a result, zos-profiling-error.c has been renamed to ibm-profiling.c. This revision also passes clang/test/Driver/aix-ld.c.

[francii]

Rebase

[francii]

Add test case

[francii]

Merge z/OS and AIX profiling tests

[francii]

Remove old test case

[francii]

Remove old test case

[francii]

Add test case descriptions

[francii]

nit

[daltenty]

LGTM, thanks!

[daltenty]

clang/lib/Driver/ToolChains/Clang.cpp

6325–6326

Actually, a question here. Does this now result in the arg being claimed even if nothing was done it? Since there are case we don't go into the error path.

[francii]

Update to claiming logic:
if -pg is passed, only claim -p if -p is passed afterwards.
Otherwise, always claim -p.

[francii]

Cleanup

[francii]

Always throw error when -p is in command.

[francii]

Add more test cases

[francii]

Rebase

[MaskRay]

clang/test/Driver/ibm-profiling.c

2

This is too verbose. Driver tests prefer packing many options on one line. The decreased number of lines actually improves readability.

[MaskRay]

clang/lib/Driver/ToolChains/Clang.cpp

6342

Use &&

[francii]

Update based on review

[francii]

Rebase

[francii]

clang/lib/Driver/ToolChains/Clang.cpp

6325–6326

I've updated the logic of this patch entirely. With these changes, using -p and -pg together now gives precedence to the option that is passed last.

Now, whenever we check for -p, we never do a blind claim. We instead manually claim -p in the following instances:

1. The user passes -p without also passing -pg: We manually claim -p, and push -pg to CmdArgs.
2. The user passes both -p and -pg, but -p is given precedence: We manually claim -p, and do nothing (since -pg is already in CmdArgs).

As a result, -p will only warn that it goes unused if it is passed before -pg (i.e. clang -p -pg).

[francii]

Fix missing bracket

[francii]

Rebase

[francii]

Rebase

[francii]

Test case nit

[daltenty]

Reopen since this was reverted

[francii]

Make -p a CC1 option.

[francii]

Fix segfault

[francii]

Don't claim when checking for object file

[francii]

Rebase

[francii]

Rebase for push

clang/lib/Driver/ToolChains/AIX.cpp

void aix::Linker::ConstructJob(Compilation &C, const JobAction &JA,
   } else {
     // Must be 64-bit, otherwise asserted already.
     CmdArgs.push_back("-b64");
     CmdArgs.push_back("-bpT:0x100000000");
     CmdArgs.push_back("-bpD:0x110000000");
   }
 
   auto getCrt0Basename = [&Args, IsArch32Bit] {
+    Arg *A = Args.getLastArgNoClaim(options::OPT_p, options::OPT_pg);
     // Enable gprofiling when "-pg" is specified.
-    if (Args.hasArg(options::OPT_pg))
+    if (A->getOption().matches(options::OPT_pg))
       return IsArch32Bit ? "gcrt0.o" : "gcrt0_64.o";
     // Enable profiling when "-p" is specified.
-    else if (Args.hasArg(options::OPT_p))
+    else if (A->getOption().matches(options::OPT_p))
       return IsArch32Bit ? "mcrt0.o" : "mcrt0_64.o";
     else
       return IsArch32Bit ? "crt0.o" : "crt0_64.o";
   };
 
   if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nostartfiles,
                    options::OPT_shared)) {
     CmdArgs.push_back(
if (!Args.hasArg(options::OPT_nostdlib, options::OPT_nodefaultlibs)) {
     if (Args.hasArg(options::OPT_pthreads, options::OPT_pthread))
       CmdArgs.push_back("-lpthreads");
 
     if (D.CCCIsCXX())
       CmdArgs.push_back("-lm");
 
     CmdArgs.push_back("-lc");
 
-    if (Args.hasArg(options::OPT_p, options::OPT_pg)) {
+    if (Args.hasArgNoClaim(options::OPT_p, options::OPT_pg)) {
       CmdArgs.push_back(Args.MakeArgString((llvm::Twine("-L") + D.SysRoot) +
                                            "/lib/profiled"));
       CmdArgs.push_back(Args.MakeArgString((llvm::Twine("-L") + D.SysRoot) +
                                            "/usr/lib/profiled"));
     }
   }
 
   const char *Exec = Args.MakeArgString(ToolChain.GetLinkerPath());

clang/lib/Driver/ToolChains/Clang.cpp

if (Arg *A = Args.getLastArg(options::OPT_mnop_mcount)) {
             << A->getAsString(Args) << TripleStr;
     }
     if (Arg *A = Args.getLastArg(options::OPT_mrecord_mcount)) {
       if (Arch == llvm::Triple::systemz)
         A->render(Args, CmdArgs);
       else
         D.Diag(diag::err_drv_unsupported_opt_for_target)
             << A->getAsString(Args) << TripleStr;
     }
-  }
-  if (Arg *A = Args.getLastArgNoClaim(options::OPT_p)) {
-    if (TC.getTriple().isOSAIX()) {
-      CmdArgs.push_back("-pg");
-    } else if (!TC.getTriple().isOSOpenBSD()) {
+
+    if (Arg *A = Args.getLastArgNoClaim(options::OPT_pg)) {
+      if (TC.getTriple().isOSzOS()) {
         D.Diag(diag::err_drv_unsupported_opt_for_target)
             << A->getAsString(Args) << TripleStr;
       }
     }
-  if (Arg *A = Args.getLastArgNoClaim(options::OPT_pg)) {
-    if (TC.getTriple().isOSzOS()) {
+    if (Arg *A = Args.getLastArgNoClaim(options::OPT_p)) {
+      if (!(TC.getTriple().isOSAIX() || TC.getTriple().isOSOpenBSD())) {
         D.Diag(diag::err_drv_unsupported_opt_for_target)
             << A->getAsString(Args) << TripleStr;
       }
     }
+    if (Arg *A = Args.getLastArgNoClaim(options::OPT_p, options::OPT_pg)) {
+      if (A->getOption().matches(options::OPT_p)) {
+        A->claim();
+        if (TC.getTriple().isOSAIX()) {
+          if (!Args.hasArgNoClaim(options::OPT_pg))
+            CmdArgs.push_back("-pg");
+        }
+      }
+    }
 
   if (Args.getLastArg(options::OPT_fapple_kext) ||
       (Args.hasArg(options::OPT_mkernel) && types::isCXX(InputType)))
     CmdArgs.push_back("-fapple-kext");
 
   Args.AddLastArg(CmdArgs, options::OPT_altivec_src_compat);
   Args.AddLastArg(CmdArgs, options::OPT_flax_vector_conversions_EQ);
   Args.AddLastArg(CmdArgs, options::OPT_fobjc_sender_dependent_dispatch);

clang/test/Driver/ibm-profiling.c

+// RUN: %clang -### 2>&1 \
+// RUN:        --target=s390x-none-zos \
+// RUN:        -S \
+// RUN:        -pg \
+// RUN:        %s \
+// RUN:        | FileCheck -check-prefix=FAIL-PG-NAME %s
+// FAIL-PG-NAME: error: unsupported option '-pg' for target 's390x-none-zos'
+
+// Check precedence: -pg is unused when passed first.
+// RUN: %clang -### 2>&1 \
+// RUN:        --target=powerpc-ibm-aix7.1.0.0 \
+// RUN:        --sysroot %S/Inputs/aix_ppc_tree \
+// RUN:        -pg \
+// RUN:        -p \
+// RUN:        %s \
+// RUN:        | FileCheck --check-prefix=CHECK %s
+// CHECK-NOT: warning: argument unused during compilation: '-p' [-Wunused-command-line-argument]
+// CHECK:     "-isysroot" "[[SYSROOT:[^"]+]]"
+// CHECK:     "[[SYSROOT]]/usr/lib{{/|\\\\}}mcrt0.o"
+// CHECK:     "-L[[SYSROOT]]/lib/profiled"
+// CHECK:     "-L[[SYSROOT]]/usr/lib/profiled"
+
+// Check precedence: -p is unused when passed first.
+// RUN: %clang -### \
+// RUN:        --target=powerpc-ibm-aix7.1.0.0 \
+// RUN:        --sysroot %S/Inputs/aix_ppc_tree \
+// RUN:        -p \
+// RUN:        -pg \
+// RUN:        %s 2>&1 \
+// RUN:        | FileCheck --check-prefix=CHECK2 %s
+// CHECK2: warning: argument unused during compilation: '-p' [-Wunused-command-line-argument]
+// CHECK2:     "-isysroot" "[[SYSROOT:[^"]+]]"
+// CHECK2:     "[[SYSROOT]]/usr/lib{{/|\\\\}}gcrt0.o"
+// CHECK2:     "-L[[SYSROOT]]/lib/profiled"
+// CHECK2:     "-L[[SYSROOT]]/usr/lib/profiled"
+
+// Check that -p is still used when not linking on AIX.
+// RUN: %clang -### 2>&1 \
+// RUN:        --target=powerpc-ibm-aix7.1.0.0 \
+// RUN:        -p \
+// RUN:        -S \
+// RUN:        %s \
+// RUN:   | FileCheck --check-prefix=CHECK3 %s
+// CHECK3-NOT: warning: argument unused during compilation: '-p'

clang/test/Driver/zos-profiling-error.c

-// RUN: %clang 2>&1 -### --target=s390x-none-zos -pg -S %s | FileCheck -check-prefix=FAIL-PG-NAME %s
-// FAIL-PG-NAME: error: unsupported option '-pg' for target 's390x-none-zos'