release/23.x: [Clang][Interpreter] Don't pass -fPIC on MSVC (#210507) - #210642
Conversation
|
@vgvassilev What do you think about merging this PR to the release branch? |
|
@llvm/pr-subscribers-clang Author: llvmbot ChangesBackport 617e4f5 Requested by: @dyung Full diff: https://github.com/llvm/llvm-project/pull/210642.diff 1 Files Affected:
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
|
|
This is needed to fix a test failure in the Windows-11-arm build which blocks it from completing successfully. |
|
Looks like a pre merge check is failing. |
1 similar comment
|
Looks like a pre merge check is failing. |
The pre-merge check looks more like an infrastructure failure to me, but I've asked @tstellar to take a look since he had previously fixed this test. On the release branch, I've just been committing fixes even with that failure, but still need a review! |
vgvassilev
left a comment
There was a problem hiding this comment.
Lgtm, if the premerge checks fail due to infra.
(cherry picked from commit 617e4f5)
Backport 617e4f5
Requested by: @dyung