[PATCH] D68401: [Hexagon] Fix clang driver to parse -mcpu/-mvXX and -march properly.
sgundapa closed this revision. sgundapa added a comment. I forgot to update the Differential Revision in the commit. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D68401/new/ https://reviews.llvm.org/D68401 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D68401: [Hexagon] Fix clang driver to parse -mcpu/-mvXX and -march properly.
sgundapa created this revision.
sgundapa added reviewers: bcain, bcahoon.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
Before this patch if we pass "-mcpu=hexagonv65 -march=hexagon" in this order,
the driver fails to figure out the correct cpu version. This patch fixed this
issue.
Repository:
rC Clang
https://reviews.llvm.org/D68401
Files:
clang/lib/Driver/ToolChains/Hexagon.cpp
clang/test/Driver/hexagon-toolchain-elf.c
Index: clang/test/Driver/hexagon-toolchain-elf.c
===
--- clang/test/Driver/hexagon-toolchain-elf.c
+++ clang/test/Driver/hexagon-toolchain-elf.c
@@ -121,6 +121,19 @@
// CHECK028-NOT: "-ffp-contract=fast"
// CHECK028: {{hexagon-link|ld}}
+// RUN: %clang -### -target hexagon-unknown-elf \
+// RUN: -ccc-install-dir %S/Inputs/hexagon_tree/Tools/bin \
+// RUN: -mcpu=hexagonv65 -march=hexagon\
+// RUN: %s 2>&1 \
+// RUN: | FileCheck -check-prefix=CHECK029 %s
+// RUN: %clang -### -target hexagon-unknown-elf \
+// RUN: -ccc-install-dir %S/Inputs/hexagon_tree/Tools/bin \
+// RUN: -mv65 -march=hexagon\
+// RUN: %s 2>&1 \
+// RUN: | FileCheck -check-prefix=CHECK029 %s
+// CHECK029: "-cc1" {{.*}} "-target-cpu" "hexagonv65"
+// CHECK029:
hexagon-link{{.*}}/Inputs/hexagon_tree/Tools/bin/../target/hexagon/lib/v65/crt0
+
//
-
// Test Linker related args
//
-
Index: clang/lib/Driver/ToolChains/Hexagon.cpp
===
--- clang/lib/Driver/ToolChains/Hexagon.cpp
+++ clang/lib/Driver/ToolChains/Hexagon.cpp
@@ -574,7 +574,7 @@
const StringRef HexagonToolChain::GetTargetCPUVersion(const ArgList &Args) {
Arg *CpuArg = nullptr;
- if (Arg *A = Args.getLastArg(options::OPT_mcpu_EQ, options::OPT_march_EQ))
+ if (Arg *A = Args.getLastArg(options::OPT_mcpu_EQ))
CpuArg = A;
StringRef CPU = CpuArg ? CpuArg->getValue() : GetDefaultCPU();
Index: clang/test/Driver/hexagon-toolchain-elf.c
===
--- clang/test/Driver/hexagon-toolchain-elf.c
+++ clang/test/Driver/hexagon-toolchain-elf.c
@@ -121,6 +121,19 @@
// CHECK028-NOT: "-ffp-contract=fast"
// CHECK028: {{hexagon-link|ld}}
+// RUN: %clang -### -target hexagon-unknown-elf \
+// RUN: -ccc-install-dir %S/Inputs/hexagon_tree/Tools/bin \
+// RUN: -mcpu=hexagonv65 -march=hexagon\
+// RUN: %s 2>&1 \
+// RUN: | FileCheck -check-prefix=CHECK029 %s
+// RUN: %clang -### -target hexagon-unknown-elf \
+// RUN: -ccc-install-dir %S/Inputs/hexagon_tree/Tools/bin \
+// RUN: -mv65 -march=hexagon\
+// RUN: %s 2>&1 \
+// RUN: | FileCheck -check-prefix=CHECK029 %s
+// CHECK029: "-cc1" {{.*}} "-target-cpu" "hexagonv65"
+// CHECK029: hexagon-link{{.*}}/Inputs/hexagon_tree/Tools/bin/../target/hexagon/lib/v65/crt0
+
// -
// Test Linker related args
// -
Index: clang/lib/Driver/ToolChains/Hexagon.cpp
===
--- clang/lib/Driver/ToolChains/Hexagon.cpp
+++ clang/lib/Driver/ToolChains/Hexagon.cpp
@@ -574,7 +574,7 @@
const StringRef HexagonToolChain::GetTargetCPUVersion(const ArgList &Args) {
Arg *CpuArg = nullptr;
- if (Arg *A = Args.getLastArg(options::OPT_mcpu_EQ, options::OPT_march_EQ))
+ if (Arg *A = Args.getLastArg(options::OPT_mcpu_EQ))
CpuArg = A;
StringRef CPU = CpuArg ? CpuArg->getValue() : GetDefaultCPU();
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D39071: [Hexagon] Fix the memory-leak in r316102
sgundapa abandoned this revision. sgundapa added a comment. This patch is already merged in a different commit Repository: rL LLVM https://reviews.llvm.org/D39071 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D39071: [Hexagon] Fix the memory-leak in r316102
sgundapa created this revision.
Repository:
rL LLVM
https://reviews.llvm.org/D39071
Files:
lib/Driver/ToolChains/Hexagon.cpp
Index: lib/Driver/ToolChains/Hexagon.cpp
===
--- lib/Driver/ToolChains/Hexagon.cpp
+++ lib/Driver/ToolChains/Hexagon.cpp
@@ -59,7 +59,8 @@
handleHVXWarnings(D, Args);
// Add the +hvx* features based on commandline flags.
- StringRef HVXFeature, HVXLength;
+ StringRef HVXFeature;
+ std::string HVXLength;
StringRef Cpu(toolchains::HexagonToolChain::GetTargetCPUVersion(Args));
// Handle -mhvx, -mhvx=, -mno-hvx, -mno-hvx-double.
@@ -87,17 +88,17 @@
if (!HasHVX)
D.Diag(diag::err_drv_invalid_hvx_length);
else if (A->getOption().matches(options::OPT_mhexagon_hvx_length_EQ))
- HVXLength = A->getValue();
+ HVXLength = StringRef(A->getValue()).lower();
else if (A->getOption().matches(options::OPT_mhexagon_hvx_double))
HVXLength = "128b";
}
// Default hvx-length based on Cpu.
else if (HasHVX)
-HVXLength = StringRef(getDefaultHvxLength(Cpu));
+HVXLength = getDefaultHvxLength(Cpu);
if (!HVXLength.empty()) {
HVXFeature =
-Args.MakeArgString(llvm::Twine("+hvx-length") + HVXLength.lower());
+Args.MakeArgString(llvm::Twine("+hvx-length") + HVXLength);
Features.push_back(HVXFeature);
}
}
Index: lib/Driver/ToolChains/Hexagon.cpp
===
--- lib/Driver/ToolChains/Hexagon.cpp
+++ lib/Driver/ToolChains/Hexagon.cpp
@@ -59,7 +59,8 @@
handleHVXWarnings(D, Args);
// Add the +hvx* features based on commandline flags.
- StringRef HVXFeature, HVXLength;
+ StringRef HVXFeature;
+ std::string HVXLength;
StringRef Cpu(toolchains::HexagonToolChain::GetTargetCPUVersion(Args));
// Handle -mhvx, -mhvx=, -mno-hvx, -mno-hvx-double.
@@ -87,17 +88,17 @@
if (!HasHVX)
D.Diag(diag::err_drv_invalid_hvx_length);
else if (A->getOption().matches(options::OPT_mhexagon_hvx_length_EQ))
- HVXLength = A->getValue();
+ HVXLength = StringRef(A->getValue()).lower();
else if (A->getOption().matches(options::OPT_mhexagon_hvx_double))
HVXLength = "128b";
}
// Default hvx-length based on Cpu.
else if (HasHVX)
-HVXLength = StringRef(getDefaultHvxLength(Cpu));
+HVXLength = getDefaultHvxLength(Cpu);
if (!HVXLength.empty()) {
HVXFeature =
-Args.MakeArgString(llvm::Twine("+hvx-length") + HVXLength.lower());
+Args.MakeArgString(llvm::Twine("+hvx-length") + HVXLength);
Features.push_back(HVXFeature);
}
}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D38852: [Hexagon] Handling of new HVX flags and target-features
This revision was automatically updated to reflect the committed changes. Closed by commit rL316102: [Hexagon] Handling of new HVX flags and target-features (authored by sgundapa). Changed prior to commit: https://reviews.llvm.org/D38852?vs=118802&id=119507#toc Repository: rL LLVM https://reviews.llvm.org/D38852 Files: cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td cfe/trunk/include/clang/Driver/Options.td cfe/trunk/lib/Basic/Targets/Hexagon.cpp cfe/trunk/lib/Basic/Targets/Hexagon.h cfe/trunk/lib/Driver/ToolChains/Clang.cpp cfe/trunk/lib/Driver/ToolChains/Hexagon.cpp cfe/trunk/lib/Driver/ToolChains/Hexagon.h cfe/trunk/test/CodeGen/hexagon-inline-asm.c cfe/trunk/test/Driver/hexagon-hvx.c cfe/trunk/test/Preprocessor/hexagon-predefines.c Index: cfe/trunk/include/clang/Driver/Options.td === --- cfe/trunk/include/clang/Driver/Options.td +++ cfe/trunk/include/clang/Driver/Options.td @@ -139,6 +139,10 @@ Group, DocName<"ARM">; def m_hexagon_Features_Group : OptionGroup<"">, Group, DocName<"Hexagon">; +// The features added by this group will not be added to target features. +// These are explicitly handled. +def m_hexagon_Features_HVX_Group : OptionGroup<"">, + Group, DocName<"Hexagon">; def m_ppc_Features_Group : OptionGroup<"">, Group, DocName<"PowerPC">; def m_wasm_Features_Group : OptionGroup<"">, @@ -2495,14 +2499,26 @@ Alias, AliasArgs<["hexagonv60"]>; def mv62 : Flag<["-"], "mv62">, Group, Alias, AliasArgs<["hexagonv62"]>; -def mhexagon_hvx : Flag<["-"], "mhvx">, Group, -Flags<[CC1Option]>, HelpText<"Enable Hexagon Vector eXtensions">; -def mno_hexagon_hvx : Flag<["-"], "mno-hvx">, Group, -Flags<[CC1Option]>, HelpText<"Disable Hexagon Vector eXtensions">; -def mhexagon_hvx_double : Flag<["-"], "mhvx-double">, Group, -Flags<[CC1Option]>, HelpText<"Enable Hexagon Double Vector eXtensions">; -def mno_hexagon_hvx_double : Flag<["-"], "mno-hvx-double">, Group, -Flags<[CC1Option]>, HelpText<"Disable Hexagon Double Vector eXtensions">; +def mhexagon_hvx : Flag<[ "-" ], "mhvx">, + Group, + HelpText<"Enable Hexagon Vector eXtensions">; +def mhexagon_hvx_EQ : Joined<[ "-" ], "mhvx=">, + Group, + HelpText<"Enable Hexagon Vector eXtensions">; +def mno_hexagon_hvx : Flag<[ "-" ], "mno-hvx">, + Group, + HelpText<"Disable Hexagon Vector eXtensions">; +def mhexagon_hvx_length_EQ : Joined<[ "-" ], "mhvx-length=">, +Group, +HelpText<"Set Hexagon Vector Length">, Values<"64B,128B">; +// hvx-double deprecrated flag. +def mhexagon_hvx_double : Flag<[ "-" ], "mhvx-double">, + Group, + HelpText<"Enable Hexagon Double Vector eXtensions">; +def mno_hexagon_hvx_double +: Flag<[ "-" ], "mno-hvx-double">, + Group, + HelpText<"Disable Hexagon Double Vector eXtensions">; // These are legacy user-facing driver-level option spellings. They are always // aliases for options that are spelled using the more common Unix / GNU flag Index: cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td === --- cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td +++ cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td @@ -263,6 +263,9 @@ def err_analyzer_config_multiple_values : Error< "analyzer-config option '%0' should contain only one '='">; +def err_drv_invalid_hvx_length : Error< + "-mhvx-length is not supported without a -mhvx/-mhvx= flag">; + def err_drv_modules_validate_once_requires_timestamp : Error< "option '-fmodules-validate-once-per-build-session' requires " "'-fbuild-session-timestamp=' or '-fbuild-session-file='">; Index: cfe/trunk/test/Preprocessor/hexagon-predefines.c === --- cfe/trunk/test/Preprocessor/hexagon-predefines.c +++ cfe/trunk/test/Preprocessor/hexagon-predefines.c @@ -1,32 +1,43 @@ // RUN: %clang_cc1 -E -dM -triple hexagon-unknown-elf -target-cpu hexagonv5 %s | FileCheck %s -check-prefix CHECK-V5 - // CHECK-V5: #define __HEXAGON_ARCH__ 5 // CHECK-V5: #define __HEXAGON_V5__ 1 +// CHECK-V5-NOT: #define __HVX_LENGTH__ +// CHECK-V5-NOT: #define __HVX__ 1 // CHECK-V5: #define __hexagon__ 1 // RUN: %clang_cc1 -E -dM -triple hexagon-unknown-elf -target-cpu hexagonv55 %s | FileCheck %s -check-prefix CHECK-V55 - // CHECK-V55: #define __HEXAGON_ARCH__ 55 // CHECK-V55: #define __HEXAGON_V55__ 1 +// CHECK-V55-NOT: #define __HVX_LENGTH__ +// CHECK-V55-NOT: #define __HVX__ 1 // CHECK-V55: #define __hexagon__ 1 // RUN: %clang_cc1 -E -dM -triple hexagon-unknown-elf -target-cpu hexagonv60 %s
[PATCH] D38852: [Hexagon] Handling of new HVX flags and target-features
sgundapa added a comment. Ping Repository: rL LLVM https://reviews.llvm.org/D38852 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D38852: [Hexagon] Handling of new HVX flags and target-features
sgundapa created this revision.
Herald added a subscriber: eraman.
This patch has the following changes
1. A new flag "-mhvx-length={64B|128B}" is introduced to specify the
length of the vector. Previously we have used "-mhvx-double" for 128 Bytes.
This adds the target-feature "+hvx-length{64|128}b"
2. The "-mhvx" flag must be provided on command line to enable HVX for Hexagon.
If no -mhvx-length flag is specified, a default length is picked from the arch
mentioned in this priority order from either -mhvx=vxx or -mcpu. For v60 and
v62
the default length is 64 Byte. For unknown versions, the length is 128 Byte.
The -mhvx flag adds the target-feature "+hvxv{hvx_version}"
3. The 64 Byte mode is soon going to be deprecated. A warning is emitted if 64
Byte
is enabled. A warning is still emitted for the default 64 Byte as well. This
warning
can be suppressed with a -Wno flag.
4. The "-mhvx-double" and "-mno-hvx-double" flags are deprecated. A warning is
emitted if the driver sees them on commandline. "-mhvx-double" is an alias to
"-mhvx-length=128B"
5. The compilation will error out if -mhvx-length is specified with out an
-mhvx/-mhvx= flag
6. The macro __HVX_LENGTH__ is defined and is set to the length of the
vector.
Eg: #define __HVX_LENGTH__ 64
7. The macro __HVX_ARCH__ is defined and is set to the version of the HVX.
Eg: #define __HVX_ARCH__ 62
Repository:
rL LLVM
https://reviews.llvm.org/D38852
Files:
include/clang/Basic/DiagnosticDriverKinds.td
include/clang/Driver/Options.td
lib/Basic/Targets/Hexagon.cpp
lib/Basic/Targets/Hexagon.h
lib/Driver/ToolChains/Clang.cpp
lib/Driver/ToolChains/Hexagon.cpp
lib/Driver/ToolChains/Hexagon.h
test/CodeGen/hexagon-inline-asm.c
test/Driver/hexagon-hvx.c
test/Preprocessor/hexagon-predefines.c
Index: test/Preprocessor/hexagon-predefines.c
===
--- test/Preprocessor/hexagon-predefines.c
+++ test/Preprocessor/hexagon-predefines.c
@@ -1,32 +1,43 @@
// RUN: %clang_cc1 -E -dM -triple hexagon-unknown-elf -target-cpu hexagonv5 %s | FileCheck %s -check-prefix CHECK-V5
-
// CHECK-V5: #define __HEXAGON_ARCH__ 5
// CHECK-V5: #define __HEXAGON_V5__ 1
+// CHECK-V5-NOT: #define __HVX_LENGTH__
+// CHECK-V5-NOT: #define __HVX__ 1
// CHECK-V5: #define __hexagon__ 1
// RUN: %clang_cc1 -E -dM -triple hexagon-unknown-elf -target-cpu hexagonv55 %s | FileCheck %s -check-prefix CHECK-V55
-
// CHECK-V55: #define __HEXAGON_ARCH__ 55
// CHECK-V55: #define __HEXAGON_V55__ 1
+// CHECK-V55-NOT: #define __HVX_LENGTH__
+// CHECK-V55-NOT: #define __HVX__ 1
// CHECK-V55: #define __hexagon__ 1
// RUN: %clang_cc1 -E -dM -triple hexagon-unknown-elf -target-cpu hexagonv60 %s | FileCheck %s -check-prefix CHECK-V60
-
// CHECK-V60: #define __HEXAGON_ARCH__ 60
// CHECK-V60: #define __HEXAGON_V60__ 1
+// CHECK-V60-NOT: #define __HVX_LENGTH__
+// CHECK-V60-NOT: #define __HVX__ 1
// CHECK-V60: #define __hexagon__ 1
-// RUN: %clang_cc1 -E -dM -triple hexagon-unknown-elf -target-cpu hexagonv60 -target-feature +hvx %s | FileCheck %s -check-prefix CHECK-V60HVX
-
-// CHECK-V60HVX: #define __HEXAGON_ARCH__ 60
-// CHECK-V60HVX: #define __HEXAGON_V60__ 1
-// CHECK-V60HVX: #define __HVX__ 1
-
-// RUN: %clang_cc1 -E -dM -triple hexagon-unknown-elf -target-cpu hexagonv60 -target-feature +hvx-double %s | FileCheck %s -check-prefix CHECK-V60HVXD
-
-// CHECK-V60HVXD: #define __HEXAGON_ARCH__ 60
-// CHECK-V60HVXD: #define __HEXAGON_V60__ 1
-// CHECK-V60HVXD: #define __HVXDBL__ 1
-// CHECK-V60HVXD: #define __HVX__ 1
-// CHECK-V60HVXD: #define __hexagon__ 1
-
+// The HVX flags are explicitly defined by the driver.
+// RUN: %clang_cc1 -E -dM -triple hexagon-unknown-elf -target-cpu hexagonv60 \
+// RUN: -target-feature +hvxv60 -target-feature +hvx-length64b %s | FileCheck \
+// RUN: %s -check-prefix CHECK-V60HVX-64B
+// CHECK-V60HVX-64B: #define __HEXAGON_ARCH__ 60
+// CHECK-V60HVX-64B: #define __HEXAGON_V60__ 1
+// CHECK-V60HVX-64B-NOT: #define __HVXDBL__ 1
+// CHECK-V60HVX-64B: #define __HVX_ARCH__ 60
+// CHECK-V60HVX-64B: #define __HVX_LENGTH__ 64
+// CHECK-V60HVX-64B: #define __HVX__ 1
+// CHECK-V60HVX-64B: #define __hexagon__ 1
+
+// RUN: %clang_cc1 -E -dM -triple hexagon-unknown-elf -target-cpu hexagonv60 \
+// RUN: -target-feature +hvxv60 -target-feature +hvx-length128b %s | FileCheck \
+// RUN: %s -check-prefix CHECK-V60HVX-128B
+// CHECK-V60HVX-128B: #define __HEXAGON_ARCH__ 60
+// CHECK-V60HVX-128B: #define __HEXAGON_V60__ 1
+// CHECK-V60HVX-128B: #define __HVXDBL__ 1
+// CHECK-V60HVX-128B: #define __HVX_ARCH__ 60
+// CHECK-V60HVX-128B: #define __HVX_LENGTH__ 128
+// CHECK-V60HVX-128B: #define __HVX__ 1
+// CHECK-V60HVX-128B: #define __hexagon__ 1
Index: test/Driver/hexagon-hvx.c
===
--- /dev/null
+++ test/Driver/hexagon-hvx.c
@@ -0,0 +1,90 @@
+// -
+// Tests for
[PATCH] D38548: Hexagon] Move getHexagonTargetFeatures to Hexagon.cpp (NFC)
This revision was automatically updated to reflect the committed changes.
Closed by commit rL314926: [Hexagon] Move getHexagonTargetFeatures to
Hexagon.cpp (NFC) (authored by sgundapa).
Changed prior to commit:
https://reviews.llvm.org/D38548?vs=117686&id=117709#toc
Repository:
rL LLVM
https://reviews.llvm.org/D38548
Files:
cfe/trunk/lib/Driver/ToolChains/Clang.cpp
cfe/trunk/lib/Driver/ToolChains/Hexagon.cpp
cfe/trunk/lib/Driver/ToolChains/Hexagon.h
Index: cfe/trunk/lib/Driver/ToolChains/Clang.cpp
===
--- cfe/trunk/lib/Driver/ToolChains/Clang.cpp
+++ cfe/trunk/lib/Driver/ToolChains/Clang.cpp
@@ -273,21 +273,6 @@
OutStrings.push_back(Args.MakeArgString(Out));
}
-static void getHexagonTargetFeatures(const ArgList &Args,
- std::vector &Features) {
- handleTargetFeaturesGroup(Args, Features,
-options::OPT_m_hexagon_Features_Group);
-
- bool UseLongCalls = false;
- if (Arg *A = Args.getLastArg(options::OPT_mlong_calls,
- options::OPT_mno_long_calls)) {
-if (A->getOption().matches(options::OPT_mlong_calls))
- UseLongCalls = true;
- }
-
- Features.push_back(UseLongCalls ? "+long-calls" : "-long-calls");
-}
-
static void getWebAssemblyTargetFeatures(const ArgList &Args,
std::vector &Features) {
handleTargetFeaturesGroup(Args, Features,
options::OPT_m_wasm_Features_Group);
@@ -349,7 +334,7 @@
x86::getX86TargetFeatures(D, Triple, Args, Features);
break;
case llvm::Triple::hexagon:
-getHexagonTargetFeatures(Args, Features);
+hexagon::getHexagonTargetFeatures(Args, Features);
break;
case llvm::Triple::wasm32:
case llvm::Triple::wasm64:
Index: cfe/trunk/lib/Driver/ToolChains/Hexagon.h
===
--- cfe/trunk/lib/Driver/ToolChains/Hexagon.h
+++ cfe/trunk/lib/Driver/ToolChains/Hexagon.h
@@ -50,6 +50,10 @@
const llvm::opt::ArgList &TCArgs,
const char *LinkingOutput) const override;
};
+
+void getHexagonTargetFeatures(const llvm::opt::ArgList &Args,
+ std::vector &Features);
+
} // end namespace hexagon.
} // end namespace tools
Index: cfe/trunk/lib/Driver/ToolChains/Hexagon.cpp
===
--- cfe/trunk/lib/Driver/ToolChains/Hexagon.cpp
+++ cfe/trunk/lib/Driver/ToolChains/Hexagon.cpp
@@ -27,6 +27,22 @@
using namespace clang;
using namespace llvm::opt;
+// Hexagon target features.
+void hexagon::getHexagonTargetFeatures(const ArgList &Args,
+ std::vector &Features) {
+ handleTargetFeaturesGroup(Args, Features,
+options::OPT_m_hexagon_Features_Group);
+
+ bool UseLongCalls = false;
+ if (Arg *A = Args.getLastArg(options::OPT_mlong_calls,
+ options::OPT_mno_long_calls)) {
+if (A->getOption().matches(options::OPT_mlong_calls))
+ UseLongCalls = true;
+ }
+
+ Features.push_back(UseLongCalls ? "+long-calls" : "-long-calls");
+}
+
// Hexagon tools start.
void hexagon::Assembler::RenderExtraToolArgs(const JobAction &JA,
ArgStringList &CmdArgs) const {
Index: cfe/trunk/lib/Driver/ToolChains/Clang.cpp
===
--- cfe/trunk/lib/Driver/ToolChains/Clang.cpp
+++ cfe/trunk/lib/Driver/ToolChains/Clang.cpp
@@ -273,21 +273,6 @@
OutStrings.push_back(Args.MakeArgString(Out));
}
-static void getHexagonTargetFeatures(const ArgList &Args,
- std::vector &Features) {
- handleTargetFeaturesGroup(Args, Features,
-options::OPT_m_hexagon_Features_Group);
-
- bool UseLongCalls = false;
- if (Arg *A = Args.getLastArg(options::OPT_mlong_calls,
- options::OPT_mno_long_calls)) {
-if (A->getOption().matches(options::OPT_mlong_calls))
- UseLongCalls = true;
- }
-
- Features.push_back(UseLongCalls ? "+long-calls" : "-long-calls");
-}
-
static void getWebAssemblyTargetFeatures(const ArgList &Args,
std::vector &Features) {
handleTargetFeaturesGroup(Args, Features, options::OPT_m_wasm_Features_Group);
@@ -349,7 +334,7 @@
x86::getX86TargetFeatures(D, Triple, Args, Features);
break;
case llvm::Triple::hexagon:
-getHexagonTargetFeatures(Args, Features);
+hexagon::getHexagonTargetFeatures(Args, Features);
break;
case llvm::Triple::wasm32:
case llvm::Triple::wasm64:
Index: cfe/trunk/lib/Driver/ToolChains/Hexagon.h
===
--- cfe/trunk/lib/Driver/ToolChains/Hexagon.h
+++ cfe/trunk/lib/Driver/ToolChains/Hexagon.h
@@ -50,6 +50,10 @@
[PATCH] D35577: Add -flookup-tables and -fno-lookup-tables flags
sgundapa added a comment. The only change that is needed is to disable lookup-tables based on the attribute "no-jump-tables" set by "-fno-jump-tables" clang flag. It implies that this change is not required. https://reviews.llvm.org/D35577 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D35578: Add -fswitch-tables and -fno-switch-tables flags
sgundapa abandoned this revision. sgundapa added a comment. Refer to https://reviews.llvm.org/D35577 as we decided to disable lookup tables under -fno-jump-tables https://reviews.llvm.org/D35578 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D35577: Add -flookup-tables and -fno-lookup-tables flags
sgundapa added a comment. Thanks. I will make change to this affect https://reviews.llvm.org/D35577 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D35577: Add -flookup-tables and -fno-lookup-tables flags
sgundapa added a comment. I am waiting for others to approve/review the decision. https://reviews.llvm.org/D35577 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D35577: Add -flookup-tables and -fno-lookup-tables flags
sgundapa added a comment. If it is okay for the reviewers, I have no problem using -fno-jump-tables to this effect. I need to update https://reviews.llvm.org/D35578 and https://reviews.llvm.org/D35579 https://reviews.llvm.org/D35577 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D35577: Add -flookup-tables and -fno-lookup-tables flags
sgundapa added a comment. > "Should this just be part of the tuning for the hexagon backend and not > options at all" This will be useful to all the backends/archs that support a tightly coupled memory. AFAIK, hexagon is not the only target that has a TCM. https://reviews.llvm.org/D35577 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D35577: Add -flookup-tables and -fno-lookup-tables flags
sgundapa added a comment. This is not going to be a temporary option https://reviews.llvm.org/D35577 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D35577: Add -flookup-tables and -fno-lookup-tables flags
sgundapa added a comment. The discussion is scattered across these patches https://reviews.llvm.org/D35578 and https://reviews.llvm.org/D35579. I will provide a brief summary here: The idea is to control the generation of data (lookup table) generated from a function, specifically when the user is not expecting it. For hexagon, there is tightly coupled memory and the customers usually place "text" in it. For functions, which generate lookup tables, it is very very expensive to read the table from a far away non-TCM data section. This option will disable the generation of lookup tables at the expense of code bloat. This is really driven by the customers of hexagon backend. https://reviews.llvm.org/D35577 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D35578: Add -fswitch-tables and -fno-switch-tables flags
sgundapa added a comment. > Moreover, why can't this determining factor be built into the compiler so the > user doesn't even have to bother. That would be a more ideal user experience. Here is a use case : For the code that stays in TCM, the customer doesn't want the data that the code refers to be outside of TCM. As kparzysz mentioned, the loads cause a huge latency and is not intended. Disabling table generation is the right thing to do here. For code that stays in regular memory, generating tables is far more efficient than a bunch of if-elses. > As an alternative solution, why not just disable the transformation in > SimplifyCFG when -fno-jump-tables is used? The underlying issue seems to be > the same (i.e., you want to avoid generating more relocations) and AFAICT > that's what -fno-jump-tables is all about.. (Admittedly, I don't know the > full history of -fno-jump-tables, so others might disagree with this > suggestion.) Jump tables are not supported by all targets but lookup tables are. Jump tables need indirect addressing mode where as a lookup table is just an array of values. This is from "man gcc" -fno-jump-tables Do not use jump tables for switch statements even where it would be more efficient than other code generation strategies. This option is of use in conjunction with -fpic or -fPIC for building code that forms part of a dynamic linker and cannot reference the address of a jump table. On some targets, jump tables do not require a GOT and this option is not needed. This will throw some background on why this option was introduced. https://reviews.llvm.org/D35578 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D35578: Add -fswitch-tables and -fno-switch-tables flags
sgundapa added a comment. > Wouldn't the fix be to make the backend deal with this, then? Either by > putting the table with the function text, or or opting out of lookup tables? > It seems that might be a better experience for the user. That is perfectly reasonable and in fact i have committed a hexagon change recently to that effect . The llvm flag hexagon-emi-lookup-tables controls the generation of lookup table for hexagon. The problem is, I don't want the users of the compiler to use a combination of front end and back end flags to get the desired result. "-fno-jump-tables -mllvm -hexagon-emit-lookup-tables=false". This could be much neater with a "-fno-jump-tables -fno-lookup-tables" or better just "-fno-switch-tables" https://reviews.llvm.org/D35578 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D35577: Add -flookup-tables and -fno-lookup-tables flags
sgundapa updated this revision to Diff 107317.
sgundapa added a comment.
Made the changes asked by reviewers
https://reviews.llvm.org/D35577
Files:
include/clang/Driver/Options.td
include/clang/Frontend/CodeGenOptions.def
lib/CodeGen/CodeGenFunction.cpp
lib/Driver/ToolChains/Clang.cpp
lib/Frontend/CompilerInvocation.cpp
test/CodeGen/nouselookuptable.c
Index: test/CodeGen/nouselookuptable.c
===
--- /dev/null
+++ test/CodeGen/nouselookuptable.c
@@ -0,0 +1,14 @@
+// RUN: %clang -S -fno-lookup-tables %s -emit-llvm -o - \
+// RUN: | FileCheck --check-prefix=NOLOOKUP %s
+// NOLOOKUP: @foo
+// NOLOOKUP: attributes #0 = {{.*}}"no-lookup-tables"="true"{{.*}}
+
+// RUN: %clang -S %s -emit-llvm -o - | FileCheck --check-prefix=LOOKUP %s
+// RUN: %clang -S -flookup-tables %s -emit-llvm -o - \
+// RUN: | FileCheck --check-prefix=LOOKUP %s
+// LOOKUP: @foo
+// LOOKUP: attributes #0 = {{.*}}"no-lookup-tables"="false"{{.*}}
+
+void foo() {
+ return;
+}
Index: lib/Frontend/CompilerInvocation.cpp
===
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -652,6 +652,8 @@
Opts.NoUseJumpTables = Args.hasArg(OPT_fno_jump_tables);
+ Opts.NoUseLookupTables = Args.hasArg(OPT_fno_lookup_tables);
+
Opts.PrepareForLTO = Args.hasArg(OPT_flto, OPT_flto_EQ);
Opts.EmitSummaryIndex = false;
if (Arg *A = Args.getLastArg(OPT_flto_EQ)) {
Index: lib/Driver/ToolChains/Clang.cpp
===
--- lib/Driver/ToolChains/Clang.cpp
+++ lib/Driver/ToolChains/Clang.cpp
@@ -2266,6 +2266,10 @@
true))
CmdArgs.push_back("-fno-jump-tables");
+ if (!Args.hasFlag(options::OPT_flookup_tables,
options::OPT_fno_lookup_tables,
+true))
+CmdArgs.push_back("-fno-lookup-tables");
+
if (!Args.hasFlag(options::OPT_fpreserve_as_comments,
options::OPT_fno_preserve_as_comments, true))
CmdArgs.push_back("-fno-preserve-as-comments");
Index: lib/CodeGen/CodeGenFunction.cpp
===
--- lib/CodeGen/CodeGenFunction.cpp
+++ lib/CodeGen/CodeGenFunction.cpp
@@ -806,6 +806,10 @@
Fn->addFnAttr("no-jump-tables",
llvm::toStringRef(CGM.getCodeGenOpts().NoUseJumpTables));
+ // Add no-lookup-tables value.
+ Fn->addFnAttr("no-lookup-tables",
+llvm::toStringRef(CGM.getCodeGenOpts().NoUseLookupTables));
+
if (getLangOpts().OpenCL) {
// Add metadata for a kernel function.
if (const FunctionDecl *FD = dyn_cast_or_null(D))
Index: include/clang/Frontend/CodeGenOptions.def
===
--- include/clang/Frontend/CodeGenOptions.def
+++ include/clang/Frontend/CodeGenOptions.def
@@ -177,6 +177,7 @@
CODEGENOPT(UnrollLoops , 1, 0) ///< Control whether loops are unrolled.
CODEGENOPT(RerollLoops , 1, 0) ///< Control whether loops are rerolled.
CODEGENOPT(NoUseJumpTables , 1, 0) ///< Set when -fno-jump-tables is enabled.
+CODEGENOPT(NoUseLookupTables , 1, 0) ///< Set when -fno-lookup-tables is
enabled.
CODEGENOPT(UnsafeFPMath , 1, 0) ///< Allow unsafe floating point optzns.
CODEGENOPT(UnwindTables , 1, 0) ///< Emit unwind tables.
CODEGENOPT(VectorizeLoop , 1, 0) ///< Run loop vectorizer.
Index: include/clang/Driver/Options.td
===
--- include/clang/Driver/Options.td
+++ include/clang/Driver/Options.td
@@ -792,6 +792,9 @@
def fjump_tables : Flag<["-"], "fjump-tables">, Group;
def fno_jump_tables : Flag<["-"], "fno-jump-tables">, Group,
Flags<[CC1Option]>,
HelpText<"Do not use jump tables for lowering switches">;
+def flookup_tables : Flag<["-"], "flookup-tables">, Group;
+def fno_lookup_tables : Flag<["-"], "fno-lookup-tables">, Group,
Flags<[CC1Option]>,
+ HelpText<"Do not use lookup tables for lowering switches">;
// Begin sanitizer flags. These should all be core options exposed in all
driver
// modes.
Index: test/CodeGen/nouselookuptable.c
===
--- /dev/null
+++ test/CodeGen/nouselookuptable.c
@@ -0,0 +1,14 @@
+// RUN: %clang -S -fno-lookup-tables %s -emit-llvm -o - \
+// RUN: | FileCheck --check-prefix=NOLOOKUP %s
+// NOLOOKUP: @foo
+// NOLOOKUP: attributes #0 = {{.*}}"no-lookup-tables"="true"{{.*}}
+
+// RUN: %clang -S %s -emit-llvm -o - | FileCheck --check-prefix=LOOKUP %s
+// RUN: %clang -S -flookup-tables %s -emit-llvm -o - \
+// RUN: | FileCheck --check-prefix=LOOKUP %s
+// LOOKUP: @foo
+// LOOKUP: attributes #0 = {{.*}}"no-lookup-tables"="false"{{.*}}
+
+void foo() {
+ return;
+}
Index: lib/Frontend/CompilerInvocation.cpp
===
--- lib/Frontend/CompilerInvocation.cpp
++
[PATCH] D35578: Add -fswitch-tables and -fno-switch-tables flags
sgundapa added a comment. I will try to address the concerns here: > What exactly is the motivation? I'm trying to narrow down the justification > for adding yet more flags. (I just typed this message in https://reviews.llvm.org/D35579) For backends with "tightly coupled memory", in scenarios where the data is far away from text pays a good amount of penalty in terms of latency. Hexagon is one such backend. The tables (both lookup and jump) which are being generated are treated as globals with internal linkage and by default will be placed in read only data. Interestingly when programmers specify the command line flag "-fno-jump-tables", they assume there is no data that goes in to other sections. In case of llvm, the attribute "no-jump-tables" has no effect on simplifyCFG which generates the lookup table. This leads me to introduce "no-lookup-tables" > Either way, it sounds like "-fno-switch-tables" is just a synonym for the > (soon-to-be-)existing options "-fno-jump-tables -fno-lookup-tables" and > therefore doesn't need to exist as a separate option Ideally I want to rename fno-jump-tables to fno-switch-tables. > LLVM backends can opt out of these kinds of tables if they're not suitable > for the target, but why would a Clang user want to do it? Often TCM memory is small enough and this needs support for both cases(generate tables and do not generate tables) https://reviews.llvm.org/D35578 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D35578: Add -fswitch-tables and -fno-switch-tables flags
sgundapa added a comment.
The switch-case statements generate two kinds of tables.
1. Jump tables
2. Lookup tables.
While the general assumption is that switch-case statements generate jump
tables, the below case generates a lookup table by late simplifycfg
int foo(int x) {
switch (x) {
case 0: return 9;
case 1: return 20;
case 2: return 14;
case 3: return 22;
case 4: return 12;
default: return 19;
}
}
generates a
@switch.table.foo = private unnamed_addr constant [5 x i32] [i32 9, i32 20, i32
14, i32 22, i32 12]
The lookup table is more an array return values as opposed to an array of
pointers in jump table.
The "-fno-XXX-flags" disable the generation of these tables.
-fno-switch-tables implies both -fno-jump-tables and -fno-lookup-tables
https://reviews.llvm.org/D35578
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
