Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[flang] Add MSC_VER and target arch defines when targeting the MSVC ABI #73250

Merged
merged 2 commits into from Dec 4, 2023
Merged

[flang] Add MSC_VER and target arch defines when targeting the MSVC ABI #73250

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
656463c
[flang] Add MSC_VER and target arch defines when targeting the MSVC ABI
DavidTruby Nov 23, 2023
50833e2
Changes for review
DavidTruby Nov 30, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
25 changes: 25 additions & 0 deletions clang/lib/Driver/ToolChains/Flang.cpp
View file
Open in desktop
Expand Up @@ -204,6 +204,30 @@ void Flang::AddAArch64TargetArgs(const ArgList &Args,
}
}

static void addVSDefines(const ToolChain &TC, const ArgList &Args,
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
static void addVSDefines(const ToolChain &TC, const ArgList &Args,
static void addMSVCDefines(const ToolChain &TC, const ArgList &Args,

?

Copy link
Member Author

Choose a reason for hiding this comment

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

I named it this way to match processVSRuntimeLibs which I stole from clang :)

ArgStringList &CmdArgs) {

unsigned ver = 0;
const VersionTuple vt = TC.computeMSVCVersion(nullptr, Args);
ver = vt.getMajor() * 10000000 + vt.getMinor().value_or(0) * 100000 +
vt.getSubminor().value_or(0);
CmdArgs.push_back(Args.MakeArgString("-D_MSC_VER=" + Twine(ver / 100000)));
CmdArgs.push_back(Args.MakeArgString("-D_MSC_FULL_VER=" + Twine(ver)));
CmdArgs.push_back(Args.MakeArgString("-D_WIN32"));

llvm::Triple triple = TC.getTriple();
if (triple.isAArch64()) {
CmdArgs.push_back("-D_M_ARM64=1");
Copy link
Contributor

Choose a reason for hiding this comment

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

Does LLVM/Flang support the _M_ARM64EC ABI?

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't believe so. It's not tested at all if it does as far as I can tell!

} else if (triple.isX86() && triple.isArch32Bit()) {
CmdArgs.push_back("-D_M_IX86=600");
} else if (triple.isX86() && triple.isArch64Bit()) {
CmdArgs.push_back("-D_M_X64=100");
} else {
llvm_unreachable(
"Flang on Windows only supports X86_32, X86_64 and AArch64");
}
Comment on lines +221 to +228
Copy link
Contributor

Choose a reason for hiding this comment

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

Flang doesn't support 32bit

llvm-project/flang/CMakeLists.txt

Line 60 in 659e401

message(FATAL_ERROR "flang isn't supported on 32 bit CPUs")

Copy link
Member Author

Choose a reason for hiding this comment

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

We don't support 32 bit hosts but I think we do support 32 bit targets

}

static void processVSRuntimeLibrary(const ToolChain &TC, const ArgList &Args,
ArgStringList &CmdArgs) {
assert(TC.getTriple().isKnownWindowsMSVCEnvironment() &&
Expand Down Expand Up @@ -322,6 +346,7 @@ void Flang::addTargetOptions(const ArgList &Args,

if (Triple.isKnownWindowsMSVCEnvironment()) {
processVSRuntimeLibrary(TC, Args, CmdArgs);
addVSDefines(TC, Args, CmdArgs);
}

// TODO: Add target specific flags, ABI, mtune option etc.
Copy link
Contributor

Choose a reason for hiding this comment

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

Please also add -D_WIN32 to indicate that this is targeting a Windows platform.

Copy link
Contributor

Choose a reason for hiding this comment

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

OTOH this could be considered out of scope for this PR.

Expand Down
10 changes: 10 additions & 0 deletions flang/test/Driver/msvc-defines.f90
View file
Open in desktop
@@ -0,0 +1,10 @@
! RUN: %flang -### --target=aarch64-windows-msvc %S/Inputs/hello.f90 -v 2>&1 | FileCheck %s --check-prefixes=MSVC,MSVC-AARCH64
! RUN: %flang -### --target=i386-windows-msvc %S/Inputs/hello.f90 -v 2>&1 | FileCheck %s --check-prefixes=MSVC,MSVC-X86_32
! RUN: %flang -### --target=x86_64-windows-msvc %S/Inputs/hello.f90 -v 2>&1 | FileCheck %s --check-prefixes=MSVC,MSVC-X86_64

! MSVC: -fc1
! MSVC-SAME: -D_MSC_VER={{[0-9]*}}
! MSVC-SAME: -D_MSC_FULL_VER={{[0-9]*}}
! MSVC-AARCH64-SAME: -D_M_ARM64=1
! MSVC-X86_32-SAME: -D_M_IX86=600
! MSVC-X86_64-SAME: -D_M_X64=100