Skip to content

[Clang][Interpreter] Don't pass -fPIC on MSVC - #210507

Merged
aokblast merged 1 commit into
mainfrom
users/aokblast/clang_rapl/fix_pic_windows
Jul 19, 2026
Merged

[Clang][Interpreter] Don't pass -fPIC on MSVC#210507
aokblast merged 1 commit into
mainfrom
users/aokblast/clang_rapl/fix_pic_windows

Conversation

@aokblast

Copy link
Copy Markdown
Contributor

MSCV does not support the -fPIC option. When it is passed, Clang falls back to PIC level 2 on x86_64 and to a static binary on AArch64. Without -fPIC, however, PIC is already enabled by default on both targets.

As a result, the PCH (built with the default PIC level by toolchain hint) and the clang-repl instance (using the fallback PIC level) end up with different PIC settings, consuming a mismatch.

Avoid passing -fPIC when targeting MSVC on Windows. This preserve the existing semantics, since PIC is already enabled by default, while ensuring that both the PCH and clang-repl use the same PIC level.

MSCV does not support the -fPIC option. When it is passed, Clang falls
back to PIC level 2 on x86_64 and to a static binary on AArch64. Without
-fPIC, however, PIC is already enabled by default on both targets.

As a result, the PCH (built with the default PIC level by toolchain
hint) and the clang-repl instance (using the fallback PIC level) end up
with different PIC settings, consuming a mismatch.

Avoid passing -fPIC when targeting MSVC on Windows. This preserve the
existing semantics, since PIC is already enabled by default, while
ensuring that both the PCH and clang-repl use the same PIC level.
@aokblast
aokblast marked this pull request as ready for review July 18, 2026 12:53
@llvmorg-github-actions llvmorg-github-actions Bot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Jul 18, 2026
@llvmorg-github-actions

Copy link
Copy Markdown

@llvm/pr-subscribers-clang

Author: aokblast

Changes

MSCV does not support the -fPIC option. When it is passed, Clang falls back to PIC level 2 on x86_64 and to a static binary on AArch64. Without -fPIC, however, PIC is already enabled by default on both targets.

As a result, the PCH (built with the default PIC level by toolchain hint) and the clang-repl instance (using the fallback PIC level) end up with different PIC settings, consuming a mismatch.

Avoid passing -fPIC when targeting MSVC on Windows. This preserve the existing semantics, since PIC is already enabled by default, while ensuring that both the PCH and clang-repl use the same PIC level.


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

1 Files Affected:

  • (modified) clang/lib/Interpreter/Interpreter.cpp (+8-2)
diff --git a/clang/lib/Interpreter/Interpreter.cpp b/clang/lib/Interpreter/Interpreter.cpp
index 933a68b50db41..0536fdcd548a2 100644
--- a/clang/lib/Interpreter/Interpreter.cpp
+++ b/clang/lib/Interpreter/Interpreter.cpp
@@ -53,6 +53,7 @@
 #include "llvm/Support/VirtualFileSystem.h"
 #include "llvm/Support/raw_ostream.h"
 #include "llvm/TargetParser/Host.h"
+#include "llvm/TargetParser/Triple.h"
 #include "llvm/Transforms/Utils/Cloning.h" // for CloneModule
 
 #define DEBUG_TYPE "clang-repl"
@@ -228,8 +229,13 @@ IncrementalCompilerBuilder::create(std::string TT,
   // host symbol may be out of range when the JIT memory is mapped more than
   // 2GB away (as on FreeBSD), breaking tests such as
   // Interpreter/simple-exception.cpp. Insert before user arguments so it can
-  // still be overridden.
-  ClangArgv.insert(ClangArgv.begin() + 1, "-fPIC");
+  // still be overridden. On Windows (excluding Cygwin/MinGW) an explicit
+  // -fPIC is an unsupported driver option that would drop non-x86_64 targets
+  // to PIC level 0; PIC is already the forced default there where relevant,
+  // so don't inject it.
+  llvm::Triple TargetTriple(TT);
+  if (!TargetTriple.isOSWindows() || TargetTriple.isOSCygMing())
+    ClangArgv.insert(ClangArgv.begin() + 1, "-fPIC");
 
   // Prepending -c to force the driver to do something if no action was
   // specified. By prepending we allow users to override the default

@aokblast
aokblast requested review from dyung, lhames and vgvassilev July 18, 2026 12:54
@aokblast

Copy link
Copy Markdown
Contributor Author

@dyung I am wondering if we can run on the breaking machine as MSVC in aarch64 is not in PR hook.

@aokblast aokblast changed the title [Clang][Interpreter] Respect default setting on Windows [Clang][Interpreter] Don't pass -fPIC on MSVC Jul 18, 2026
@dyung

dyung commented Jul 18, 2026

Copy link
Copy Markdown
Contributor

@dyung I am wondering if we can run on the breaking machine as MSVC in aarch64 is not in PR hook.

I ran your changes on the failing bot and they seemed to work: https://lab.llvm.org/buildbot/#/builders/161/builds/12358

@vgvassilev vgvassilev left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lgtm!

@aokblast
aokblast merged commit 617e4f5 into main Jul 19, 2026
18 checks passed
@aokblast
aokblast deleted the users/aokblast/clang_rapl/fix_pic_windows branch July 19, 2026 00:32
@github-project-automation github-project-automation Bot moved this from Needs Triage to Done in LLVM Release Status Jul 19, 2026
@aokblast

Copy link
Copy Markdown
Contributor Author

Thanks!

@dyung dyung moved this from Done to Needs Backport PR in LLVM Release Status Jul 20, 2026
@dyung

dyung commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

/cherry-pick 617e4f5

@llvmbot

llvmbot commented Jul 20, 2026

Copy link
Copy Markdown
Member

/pull-request #210642

@llvmbot llvmbot moved this from Needs Backport PR to Done in LLVM Release Status Jul 20, 2026
dyung pushed a commit to llvmbot/llvm-project that referenced this pull request Jul 22, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category

Projects

Development

Successfully merging this pull request may close these issues.

4 participants