[PATCH] D51806: [clang-cl] Enable -march
This revision was automatically updated to reflect the committed changes. Closed by commit rC341847: [clang-cl] Enable -march option (authored by aganea, committed by ). Repository: rC Clang https://reviews.llvm.org/D51806 Files: include/clang/Driver/Options.td test/Driver/cl-options.c Index: include/clang/Driver/Options.td === --- include/clang/Driver/Options.td +++ include/clang/Driver/Options.td @@ -1931,7 +1931,7 @@ def mwatchos_version_min_EQ : Joined<["-"], "mwatchos-version-min=">, Group; def mwatchos_simulator_version_min_EQ : Joined<["-"], "mwatchos-simulator-version-min=">; def mwatchsimulator_version_min_EQ : Joined<["-"], "mwatchsimulator-version-min=">, Alias; -def march_EQ : Joined<["-"], "march=">, Group; +def march_EQ : Joined<["-"], "march=">, Group, Flags<[CoreOption]>; def masm_EQ : Joined<["-"], "masm=">, Group, Flags<[DriverOption]>; def mcmodel_EQ : Joined<["-"], "mcmodel=">, Group; def mimplicit_it_EQ : Joined<["-"], "mimplicit-it=">, Group; Index: test/Driver/cl-options.c === --- test/Driver/cl-options.c +++ test/Driver/cl-options.c @@ -610,6 +610,7 @@ // RUN: -flto \ // RUN: -fmerge-all-constants \ // RUN: -no-canonical-prefixes \ +// RUN: -march=skylake \ // RUN: --version \ // RUN: -Werror /Zs -- %s 2>&1 Index: include/clang/Driver/Options.td === --- include/clang/Driver/Options.td +++ include/clang/Driver/Options.td @@ -1931,7 +1931,7 @@ def mwatchos_version_min_EQ : Joined<["-"], "mwatchos-version-min=">, Group; def mwatchos_simulator_version_min_EQ : Joined<["-"], "mwatchos-simulator-version-min=">; def mwatchsimulator_version_min_EQ : Joined<["-"], "mwatchsimulator-version-min=">, Alias; -def march_EQ : Joined<["-"], "march=">, Group; +def march_EQ : Joined<["-"], "march=">, Group, Flags<[CoreOption]>; def masm_EQ : Joined<["-"], "masm=">, Group, Flags<[DriverOption]>; def mcmodel_EQ : Joined<["-"], "mcmodel=">, Group; def mimplicit_it_EQ : Joined<["-"], "mimplicit-it=">, Group; Index: test/Driver/cl-options.c === --- test/Driver/cl-options.c +++ test/Driver/cl-options.c @@ -610,6 +610,7 @@ // RUN: -flto \ // RUN: -fmerge-all-constants \ // RUN: -no-canonical-prefixes \ +// RUN: -march=skylake \ // RUN: --version \ // RUN: -Werror /Zs -- %s 2>&1 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
RE: [PATCH] D51806: [clang-cl] Enable -march
Yes, indeed. The code in clang/lib/Driver/ToolChains/Arch/X86.cpp makes that –march is always parsed, leaving out /arch unused, no matter in which order they appear. De : Nico Weber Envoyé : 10 septembre 2018 12:14 À : reviews+d51806+public+c25b17a1aa94d...@reviews.llvm.org; Senthil Kumar Selvaraj via Phabricator Cc : Alexandre Ganea ; cfe-commits Objet : Re: [PATCH] D51806: [clang-cl] Enable -march On Mon, Sep 10, 2018 at 10:33 AM Hans Wennborg via Phabricator via cfe-commits mailto:cfe-commits@lists.llvm.org>> wrote: hans added a comment. In https://reviews.llvm.org/D51806#1228893, @aganea wrote: > @hans Just an after thought: maybe we should prevent usage of `-march=` and > `/arch:` at the same time. What do you think? I can add another patch for > that purpose. Hmm, yes, at least we should warn or do something smart. Currently it doesn't look like they'd interact well together in x86::getX86TargetCPU Wouldn't you get an "unused arg" for /arch if you use -march and /arch at the same time? Repository: rC Clang https://reviews.llvm.org/D51806 ___ cfe-commits mailing list cfe-commits@lists.llvm.org<mailto:cfe-commits@lists.llvm.org> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D51806: [clang-cl] Enable -march
On Mon, Sep 10, 2018 at 10:33 AM Hans Wennborg via Phabricator via cfe-commits wrote: > hans added a comment. > > In https://reviews.llvm.org/D51806#1228893, @aganea wrote: > > > @hans Just an after thought: maybe we should prevent usage of `-march=` > and `/arch:` at the same time. What do you think? I can add another patch > for that purpose. > > > Hmm, yes, at least we should warn or do something smart. Currently it > doesn't look like they'd interact well together in x86::getX86TargetCPU > Wouldn't you get an "unused arg" for /arch if you use -march and /arch at the same time? > > > Repository: > rC Clang > > https://reviews.llvm.org/D51806 > > > > ___ > cfe-commits mailing list > cfe-commits@lists.llvm.org > http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits > ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51806: [clang-cl] Enable -march
hans added a comment. In https://reviews.llvm.org/D51806#1228893, @aganea wrote: > @hans Just an after thought: maybe we should prevent usage of `-march=` and > `/arch:` at the same time. What do you think? I can add another patch for > that purpose. Hmm, yes, at least we should warn or do something smart. Currently it doesn't look like they'd interact well together in x86::getX86TargetCPU Repository: rC Clang https://reviews.llvm.org/D51806 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51806: [clang-cl] Enable -march
aganea added a comment. @hans Just an after thought: maybe we should prevent usage of `-march=` and `/arch:` at the same time. What do you think? I can add another patch for that purpose. Repository: rC Clang https://reviews.llvm.org/D51806 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51806: [clang-cl] Enable -march
aganea added a comment. Thank you! Repository: rC Clang https://reviews.llvm.org/D51806 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51806: [clang-cl] Enable -march
hans accepted this revision. hans added a comment. This revision is now accepted and ready to land. lgtm Repository: rC Clang https://reviews.llvm.org/D51806 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51806: [clang-cl] Enable -march
aganea updated this revision to Diff 164666. aganea added a comment. Updated diff with coverage test as requested. Repository: rC Clang https://reviews.llvm.org/D51806 Files: include/clang/Driver/Options.td test/Driver/cl-options.c Index: test/Driver/cl-options.c === --- test/Driver/cl-options.c +++ test/Driver/cl-options.c @@ -610,6 +610,7 @@ // RUN: -flto \ // RUN: -fmerge-all-constants \ // RUN: -no-canonical-prefixes \ +// RUN: -march=skylake \ // RUN: --version \ // RUN: -Werror /Zs -- %s 2>&1 Index: include/clang/Driver/Options.td === --- include/clang/Driver/Options.td +++ include/clang/Driver/Options.td @@ -1931,7 +1931,7 @@ def mwatchos_version_min_EQ : Joined<["-"], "mwatchos-version-min=">, Group; def mwatchos_simulator_version_min_EQ : Joined<["-"], "mwatchos-simulator-version-min=">; def mwatchsimulator_version_min_EQ : Joined<["-"], "mwatchsimulator-version-min=">, Alias; -def march_EQ : Joined<["-"], "march=">, Group; +def march_EQ : Joined<["-"], "march=">, Group, Flags<[CoreOption]>; def masm_EQ : Joined<["-"], "masm=">, Group, Flags<[DriverOption]>; def mcmodel_EQ : Joined<["-"], "mcmodel=">, Group; def mimplicit_it_EQ : Joined<["-"], "mimplicit-it=">, Group; Index: test/Driver/cl-options.c === --- test/Driver/cl-options.c +++ test/Driver/cl-options.c @@ -610,6 +610,7 @@ // RUN: -flto \ // RUN: -fmerge-all-constants \ // RUN: -no-canonical-prefixes \ +// RUN: -march=skylake \ // RUN: --version \ // RUN: -Werror /Zs -- %s 2>&1 Index: include/clang/Driver/Options.td === --- include/clang/Driver/Options.td +++ include/clang/Driver/Options.td @@ -1931,7 +1931,7 @@ def mwatchos_version_min_EQ : Joined<["-"], "mwatchos-version-min=">, Group; def mwatchos_simulator_version_min_EQ : Joined<["-"], "mwatchos-simulator-version-min=">; def mwatchsimulator_version_min_EQ : Joined<["-"], "mwatchsimulator-version-min=">, Alias; -def march_EQ : Joined<["-"], "march=">, Group; +def march_EQ : Joined<["-"], "march=">, Group, Flags<[CoreOption]>; def masm_EQ : Joined<["-"], "masm=">, Group, Flags<[DriverOption]>; def mcmodel_EQ : Joined<["-"], "mcmodel=">, Group; def mimplicit_it_EQ : Joined<["-"], "mimplicit-it=">, Group; ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51806: [clang-cl] Enable -march
hans added a comment. Okay, that sounds good to me. Please also add a test for this in test/Driver/cl-options.c in the "Accept "core" clang options" section. Repository: rC Clang https://reviews.llvm.org/D51806 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51806: [clang-cl] Enable -march
aganea added a comment. @hans I want to target precisely a given CPU when using `clang-cl`. The options in the `m_x86_Features_Group` are too granular. It is easier to just add `-march=skylake` on the cmd-line. Is there any other way? Repository: rC Clang https://reviews.llvm.org/D51806 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51806: [clang-cl] Enable -march
hans added a comment. All flags in the m_x86_Features_Group, i.e. -msse, -mavx and so on are all supported. I'm just curious to hear what it is that you need from -march? Repository: rC Clang https://reviews.llvm.org/D51806 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51806: [clang-cl] Enable -march
aganea created this revision. aganea added reviewers: thakis, ddunbar, hans. Herald added a subscriber: cfe-commits. Currently, `-march` does not work in the `clang-cl` driver. We are currently migrating from MSVC. The project size makes that we have to stick with `clang-cl` for a little while. However, we don't want to loose optimisation oportunities in the runtime code, and the MSVC flag `/arch` only supports a limited set of options. Repository: rC Clang https://reviews.llvm.org/D51806 Files: include/clang/Driver/Options.td Index: include/clang/Driver/Options.td === --- include/clang/Driver/Options.td +++ include/clang/Driver/Options.td @@ -1931,7 +1931,7 @@ def mwatchos_version_min_EQ : Joined<["-"], "mwatchos-version-min=">, Group; def mwatchos_simulator_version_min_EQ : Joined<["-"], "mwatchos-simulator-version-min=">; def mwatchsimulator_version_min_EQ : Joined<["-"], "mwatchsimulator-version-min=">, Alias; -def march_EQ : Joined<["-"], "march=">, Group; +def march_EQ : Joined<["-"], "march=">, Group, Flags<[CoreOption]>; def masm_EQ : Joined<["-"], "masm=">, Group, Flags<[DriverOption]>; def mcmodel_EQ : Joined<["-"], "mcmodel=">, Group; def mimplicit_it_EQ : Joined<["-"], "mimplicit-it=">, Group; Index: include/clang/Driver/Options.td === --- include/clang/Driver/Options.td +++ include/clang/Driver/Options.td @@ -1931,7 +1931,7 @@ def mwatchos_version_min_EQ : Joined<["-"], "mwatchos-version-min=">, Group; def mwatchos_simulator_version_min_EQ : Joined<["-"], "mwatchos-simulator-version-min=">; def mwatchsimulator_version_min_EQ : Joined<["-"], "mwatchsimulator-version-min=">, Alias; -def march_EQ : Joined<["-"], "march=">, Group; +def march_EQ : Joined<["-"], "march=">, Group, Flags<[CoreOption]>; def masm_EQ : Joined<["-"], "masm=">, Group, Flags<[DriverOption]>; def mcmodel_EQ : Joined<["-"], "mcmodel=">, Group; def mimplicit_it_EQ : Joined<["-"], "mimplicit-it=">, Group; ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
