[clang][modules] Fix CodeGen options that can affect the AST.

[ributzka]

OptimizationLevel and OptimizeSize can affect the generated AST. They
indirectly affect the Optimize and OptimizeSize frontend options, which in
turn set predefined macro definitions.

This fixes rdar://121228252.

[llvmbot]

@llvm/pr-subscribers-clang

Author: Juergen Ributzka (ributzka)

Changes

OptimizationLevel and OptimizeSize can affect the generated AST. They
indirectly affect the Optimize and OptimizeSize frontend options, which in
turn set predefined macro definitions.

This fixes rdar://121228252.

---

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

3 Files Affected:

• (modified) clang/include/clang/Basic/CodeGenOptions.def (+12-2)
• (modified) clang/lib/Basic/CodeGenOptions.cpp (+2)
• (modified) clang/test/ClangScanDeps/strip-codegen-args.m (+7-4)

diff --git a/clang/include/clang/Basic/CodeGenOptions.def b/clang/include/clang/Basic/CodeGenOptions.def
index 2c4fb6745bc172..f535291b6ba4ee 100644
--- a/clang/include/clang/Basic/CodeGenOptions.def
+++ b/clang/include/clang/Basic/CodeGenOptions.def
@@ -12,6 +12,9 @@
 // that have enumeration type and VALUE_CODEGENOPT is a code
 // generation option that describes a value rather than a flag.
 //
+// AFFECTING_VALUE_CODEGENOPT is used for code generation options that can
+// affect the AST.
+//
 //===----------------------------------------------------------------------===//
 #ifndef CODEGENOPT
 #  error Define the CODEGENOPT macro to handle language options
@@ -27,6 +30,11 @@ CODEGENOPT(Name, Bits, Default)
 CODEGENOPT(Name, Bits, Default)
 #endif
 
+#ifndef AFFECTING_VALUE_CODEGENOPT
+#  define AFFECTING_VALUE_CODEGENOPT(Name, Bits, Default) \
+VALUE_CODEGENOPT(Name, Bits, Default)
+#endif
+
 CODEGENOPT(DisableIntegratedAS, 1, 0) ///< -no-integrated-as
 CODEGENOPT(RelaxELFRelocations, 1, 1) ///< -Wa,-mrelax-relocations={yes,no}
 CODEGENOPT(AsmVerbose        , 1, 0) ///< -dA, -fverbose-asm.
@@ -193,8 +201,10 @@ ENUM_CODEGENOPT(ObjCDispatchMethod, ObjCDispatchMethodKind, 2, Legacy)
 CODEGENOPT(ObjCConvertMessagesToRuntimeCalls , 1, 1)
 CODEGENOPT(ObjCAvoidHeapifyLocalBlocks, 1, 0)
 
-VALUE_CODEGENOPT(OptimizationLevel, 2, 0) ///< The -O[0-3] option specified.
-VALUE_CODEGENOPT(OptimizeSize, 2, 0) ///< If -Os (==1) or -Oz (==2) is specified.
+
+// The optimization options affect frontend options, whicn in turn do affect the AST.
+AFFECTING_VALUE_CODEGENOPT(OptimizationLevel, 2, 0) ///< The -O[0-3] option specified.
+AFFECTING_VALUE_CODEGENOPT(OptimizeSize, 2, 0) ///< If -Os (==1) or -Oz (==2) is specified.
 
 CODEGENOPT(AtomicProfileUpdate , 1, 0) ///< Set -fprofile-update=atomic
 /// Choose profile instrumenation kind or no instrumentation.
diff --git a/clang/lib/Basic/CodeGenOptions.cpp b/clang/lib/Basic/CodeGenOptions.cpp
index 182d0a2fa4d88f..79d715305ef20b 100644
--- a/clang/lib/Basic/CodeGenOptions.cpp
+++ b/clang/lib/Basic/CodeGenOptions.cpp
@@ -27,6 +27,8 @@ void CodeGenOptions::resetNonModularOptions(StringRef ModuleFormat) {
 #define ENUM_DEBUGOPT(Name, Type, Bits, Default)
 #define CODEGENOPT(Name, Bits, Default) Name = Default;
 #define ENUM_CODEGENOPT(Name, Type, Bits, Default) set##Name(Default);
+// Do not reset AST affecting code generation options.
+#define AFFECTING_VALUE_CODEGENOPT(Name, Bits, Default)
 #include "clang/Basic/CodeGenOptions.def"
 
   // Next reset all debug options that can always be reset, because they never
diff --git a/clang/test/ClangScanDeps/strip-codegen-args.m b/clang/test/ClangScanDeps/strip-codegen-args.m
index bb7e76e86aa2f9..71171f4983386d 100644
--- a/clang/test/ClangScanDeps/strip-codegen-args.m
+++ b/clang/test/ClangScanDeps/strip-codegen-args.m
@@ -5,11 +5,14 @@
 // RUN: clang-scan-deps -compilation-database %t/cdb1.json -format experimental-full > %t/result1.txt
 // RUN: FileCheck %s -input-file %t/result1.txt
 
-// This tests that codegen option that do not affect the AST or generation of a module are removed. 
+// This tests that codegen option that do not affect the AST or generation of a
+// module are removed. It also tests that the optimization options that affect
+// the AST are not reset to -O0.
 
 // CHECK:        "modules": [
 // CHECK-NEXT:     {
 // CHECK:            "command-line": [
+// CHECK-NOT:          "-O0"
 // CHECK-NOT:          "-flto"
 // CHECK-NOT:          "-fno-autolink"
 // CHECK-NOT:          "-mrelax-relocations=no"
@@ -23,17 +26,17 @@
 [
   {
     "directory": "DIR",
-    "command": "clang -Imodules/A -fmodules -fmodules-cache-path=DIR/module-cache -fimplicit-modules -flto -fno-autolink -Xclang -mrelax-relocations=no -fsyntax-only DIR/t1.m",
+    "command": "clang -Imodules/A -fmodules -fmodules-cache-path=DIR/module-cache -fimplicit-modules -O2 -flto -fno-autolink -Xclang -mrelax-relocations=no -fsyntax-only DIR/t1.m",
     "file": "DIR/t1.m"
   },
   {
     "directory": "DIR",
-    "command": "clang -Imodules/A -fmodules -fmodules-cache-path=DIR/module-cache -fimplicit-modules -flto=thin -fautolink -fsyntax-only DIR/t2.m",
+    "command": "clang -Imodules/A -fmodules -fmodules-cache-path=DIR/module-cache -fimplicit-modules -O2 -flto=thin -fautolink -fsyntax-only DIR/t2.m",
     "file": "DIR/t2.m"
   },
   {
     "directory": "DIR",
-    "command": "clang -Imodules/A -fmodules -fmodules-cache-path=DIR/module-cache -fimplicit-modules -flto=full -fsyntax-only DIR/t3.m",
+    "command": "clang -Imodules/A -fmodules -fmodules-cache-path=DIR/module-cache -fimplicit-modules -O2 -flto=full -fsyntax-only DIR/t3.m",
     "file": "DIR/t2.m"
   }
 ]

[Bigcheese]

LGTM, although are we sure none of the other options should be affecting? I just did a quick search and it seems like this is it.

I don't think it matters here, but the actual option that controls the macro is on LangOptions and where it's set has a "FIXME: Eliminate this dependency" where it directly parses the command line. As long as we're not changing the value the LangOptions version should always match.

[ributzka]

LGTM, although are we sure none of the other options should be affecting? I just did a quick search and it seems like this is it.

I don't think it matters here, but the actual option that controls the macro is on LangOptions and where it's set has a "FIXME: Eliminate this dependency" where it directly parses the command line. As long as we're not changing the value the LangOptions version should always match.

I checked every usage of all codegen options when I did the original change, but I missed the double parsing of the command line argument in LangOpts. That was not a pattern I was looking for.