[clang] Move opt level in clang toolchain to clang::ConstructJob start

[omarahmed1111]

We currently transfer the opt level from the user clang call to CC1 args at the end of the ConstructJob function, this might lead to bugs as ConstructJob is a big function and we easily could add a change that would return early from it. That would cause the opt level to not be transferred to CC1 args and lead to wrong opt level compilation and would be hard to spot. This PR moves the opt level to the beginning of the function as opt level should be a direct transfer without any problems, it also removes the redundancy where it was added 2 times through the function.

[llvmbot]

@llvm/pr-subscribers-clang

@llvm/pr-subscribers-clang-driver

Author: Omar Ahmed (omarahmed1111)

Changes

We currently transfer the opt level from the user clang call to CC1 args at the end of the ConstructJob function, this might lead to bugs as ConstructJob is a big function and we easily could add a change that would return early from it. That would cause the opt level to not be transferred to CC1 args and lead to wrong opt level compilation and would be hard to spot. This PR moves the opt level to the beginning of the function as opt level should be a direct transfer without any problems, it also removes the redundancy where it was added 2 times through the function.

---

Full diff: https://github.com/llvm/llvm-project/pull/141036.diff

1 Files Affected:

• (modified) clang/lib/Driver/ToolChains/Clang.cpp (+10-20)

diff --git a/clang/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp
index 355a48be9f493..30ecbf865d261 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -5202,6 +5202,16 @@ void Clang::ConstructJob(Compilation &C, const JobAction &JA,
     }
   }
 
+  // Optimization level for CodeGen.
+  if (const Arg *A = Args.getLastArg(options::OPT_O_Group)) {
+    if (A->getOption().matches(options::OPT_O4)) {
+      CmdArgs.push_back("-O3");
+      D.Diag(diag::warn_O4_is_O3);
+    } else {
+      A->render(Args, CmdArgs);
+    }
+  }
+
   // Unconditionally claim the printf option now to avoid unused diagnostic.
   if (const Arg *PF = Args.getLastArg(options::OPT_mprintf_kind_EQ))
     PF->claim();
@@ -5573,16 +5583,6 @@ void Clang::ConstructJob(Compilation &C, const JobAction &JA,
       break;
     }
 
-    // Optimization level for CodeGen.
-    if (const Arg *A = Args.getLastArg(options::OPT_O_Group)) {
-      if (A->getOption().matches(options::OPT_O4)) {
-        CmdArgs.push_back("-O3");
-        D.Diag(diag::warn_O4_is_O3);
-      } else {
-        A->render(Args, CmdArgs);
-      }
-    }
-
     // Input/Output file.
     if (Output.getType() == types::TY_Dependencies) {
       // Handled with other dependency code.
@@ -6463,16 +6463,6 @@ void Clang::ConstructJob(Compilation &C, const JobAction &JA,
   // preprocessed inputs and configure concludes that -fPIC is not supported.
   Args.ClaimAllArgs(options::OPT_D);
 
-  // Manually translate -O4 to -O3; let clang reject others.
-  if (Arg *A = Args.getLastArg(options::OPT_O_Group)) {
-    if (A->getOption().matches(options::OPT_O4)) {
-      CmdArgs.push_back("-O3");
-      D.Diag(diag::warn_O4_is_O3);
-    } else {
-      A->render(Args, CmdArgs);
-    }
-  }
-
   // Warn about ignored options to clang.
   for (const Arg *A :
        Args.filtered(options::OPT_clang_ignored_gcc_optimization_f_Group)) {

[omarahmed1111]

@tarunprabhu @florianhumblot @alexey-bataev @RKSimon @phoebewang Not sure who to as for review, so please review or if you could mention who is responsible for reviewing this code, Thanks!

[tarunprabhu]

Thanks. Any cleanup of this code is helpful.

[omarahmed1111]

@Artem-B @tarunprabhu Could you merge it for me as I don't have access, Thanks!