* [PATCH 00/12] aarch64: Extend aarch64_feature_flags to 128 bits
@ 2024-05-14 14:54 Andrew Carlotti
2024-05-14 14:55 ` [PATCH 01/12] aarch64: Remove unused global aarch64_tune_flags Andrew Carlotti
` (12 more replies)
0 siblings, 13 replies; 16+ messages in thread
From: Andrew Carlotti @ 2024-05-14 14:54 UTC (permalink / raw)
To: gcc-patches; +Cc: Richard Sandiford
The end goal of the series is to change the definition of aarch64_feature_flags
from a uint64_t typedef to a class with 128 bits of storage. This class uses
operator overloading to mimic the existing integer interface as much as
possible, but with added restrictions to facilate type checking and
extensibility.
Patches 01-10 are preliminary enablement work, and have passed regression
testing. Are these ok for master?
Patch 11 is an RFC, and the only patch that touches the middle end. I am
seeking clarity on which part(s) of the compiler should be expected to handle
or prevent non-bool types in instruction pattern conditions. The actual patch
does not compile by itself (though it does in combination with 12/12), but that
is not important to the questions I'm asking.
Patch 12 is then a small patch that actually replaces the uint64_t typedef with
a class. I think this patch is fine in it's current form, but it depends on a
resolution to the issues in patch 11/12 first.
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 01/12] aarch64: Remove unused global aarch64_tune_flags
2024-05-14 14:54 [PATCH 00/12] aarch64: Extend aarch64_feature_flags to 128 bits Andrew Carlotti
@ 2024-05-14 14:55 ` Andrew Carlotti
2024-05-14 14:55 ` [PATCH 02/12] aarch64: Move AARCH64_NUM_ISA_MODES definition Andrew Carlotti
` (11 subsequent siblings)
12 siblings, 0 replies; 16+ messages in thread
From: Andrew Carlotti @ 2024-05-14 14:55 UTC (permalink / raw)
To: gcc-patches; +Cc: Richard Sandiford
gcc/ChangeLog:
* config/aarch64/aarch64.cc
(aarch64_tune_flags): Remove unused global variable.
(aarch64_override_options_internal): Remove dead assignment.
diff --git a/gcc/config/aarch64/aarch64.cc b/gcc/config/aarch64/aarch64.cc
index 662ff5a9b0c715d0cab0ae4ba63af1b3c8ebbd00..4e6ad1023f638c9756ee9503b1ecbd3c1573871a 100644
--- a/gcc/config/aarch64/aarch64.cc
+++ b/gcc/config/aarch64/aarch64.cc
@@ -349,9 +349,6 @@ static bool aarch64_print_address_internal (FILE*, machine_mode, rtx,
/* The processor for which instructions should be scheduled. */
enum aarch64_processor aarch64_tune = cortexa53;
-/* Mask to specify which instruction scheduling options should be used. */
-uint64_t aarch64_tune_flags = 0;
-
/* Global flag for PC relative loads. */
bool aarch64_pcrelative_literal_loads;
@@ -18237,7 +18234,6 @@ void
aarch64_override_options_internal (struct gcc_options *opts)
{
const struct processor *tune = aarch64_get_tune_cpu (opts->x_selected_tune);
- aarch64_tune_flags = tune->flags;
aarch64_tune = tune->sched_core;
/* Make a copy of the tuning parameters attached to the core, which
we may later overwrite. */
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 02/12] aarch64: Move AARCH64_NUM_ISA_MODES definition
2024-05-14 14:54 [PATCH 00/12] aarch64: Extend aarch64_feature_flags to 128 bits Andrew Carlotti
2024-05-14 14:55 ` [PATCH 01/12] aarch64: Remove unused global aarch64_tune_flags Andrew Carlotti
@ 2024-05-14 14:55 ` Andrew Carlotti
2024-05-14 14:56 ` [PATCH 03/12] aarch64: Don't use 0 for aarch64_feature_flags Andrew Carlotti
` (10 subsequent siblings)
12 siblings, 0 replies; 16+ messages in thread
From: Andrew Carlotti @ 2024-05-14 14:55 UTC (permalink / raw)
To: gcc-patches; +Cc: Richard Sandiford
AARCH64_NUM_ISA_MODES will be used within aarch64-opts.h in a later
commit.
gcc/ChangeLog:
* config/aarch64/aarch64.h (DEF_AARCH64_ISA_MODE): Move to...
* config/aarch64/aarch64-opts.h (DEF_AARCH64_ISA_MODE): ...here.
diff --git a/gcc/config/aarch64/aarch64-opts.h b/gcc/config/aarch64/aarch64-opts.h
index a05c0d3ded1c69802f15eebb8c150c7dcc62b4ef..06a4fed3833482543891b4f7c778933f7cebd631 100644
--- a/gcc/config/aarch64/aarch64-opts.h
+++ b/gcc/config/aarch64/aarch64-opts.h
@@ -24,6 +24,11 @@
#ifndef USED_FOR_TARGET
typedef uint64_t aarch64_feature_flags;
+
+constexpr unsigned int AARCH64_NUM_ISA_MODES = (0
+#define DEF_AARCH64_ISA_MODE(IDENT) + 1
+#include "aarch64-isa-modes.def"
+);
#endif
/* The various cores that implement AArch64. */
diff --git a/gcc/config/aarch64/aarch64.h b/gcc/config/aarch64/aarch64.h
index 4fa1dfc79065c291ee5c97cc8f641c1f7c9919ec..8eb21cfcfc1e80bef051c571ec7cfae47e3393ed 100644
--- a/gcc/config/aarch64/aarch64.h
+++ b/gcc/config/aarch64/aarch64.h
@@ -189,11 +189,6 @@ enum class aarch64_feature : unsigned char {
constexpr auto AARCH64_FL_SM_STATE = AARCH64_FL_SM_ON | AARCH64_FL_SM_OFF;
-constexpr unsigned int AARCH64_NUM_ISA_MODES = (0
-#define DEF_AARCH64_ISA_MODE(IDENT) + 1
-#include "aarch64-isa-modes.def"
-);
-
/* The mask of all ISA modes. */
constexpr auto AARCH64_FL_ISA_MODES
= (aarch64_feature_flags (1) << AARCH64_NUM_ISA_MODES) - 1;
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 03/12] aarch64: Don't use 0 for aarch64_feature_flags
2024-05-14 14:54 [PATCH 00/12] aarch64: Extend aarch64_feature_flags to 128 bits Andrew Carlotti
2024-05-14 14:55 ` [PATCH 01/12] aarch64: Remove unused global aarch64_tune_flags Andrew Carlotti
2024-05-14 14:55 ` [PATCH 02/12] aarch64: Move AARCH64_NUM_ISA_MODES definition Andrew Carlotti
@ 2024-05-14 14:56 ` Andrew Carlotti
2024-05-14 14:56 ` [PATCH 04/12] aarch64: Don't compare aarch64_feature_flags to 0 Andrew Carlotti
` (9 subsequent siblings)
12 siblings, 0 replies; 16+ messages in thread
From: Andrew Carlotti @ 2024-05-14 14:56 UTC (permalink / raw)
To: gcc-patches; +Cc: Richard Sandiford
Replace all uses of 0 for aarch64_feature_flags variable initialisation
with the (almost) new macro AARCH64_NO_FEATURES.
This is needed because a later commit will disallow casts to
aarch64_feature_flags from integer types.
gcc/ChangeLog:
* common/config/aarch64/aarch64-common.cc
(all_extensions): Use AARCH64_NO_FEATURES.
(all_cores): Ditto.
(all_architectures): Ditto.
(aarch64_get_extension_string_for_isa_flags): Ditto.
* config/aarch64/aarch64-feature-deps.h (get_flags): Ditto.
(get_enable): Ditto.
(get_flags_off): Ditto.
* config/aarch64/aarch64-opts.h (AARCH64_NO_FEATURES): Define.
* config/aarch64/aarch64-protos.h: Use AARCH64_NO_FEATURES.
* config/aarch64/aarch64-sve-builtins-sme.def
(REQUIRED_EXTENSIONS): Ditto.
* config/aarch64/aarch64-sve-builtins.cc
(function_groups): Ditto.
* config/aarch64/aarch64-sve-builtins.h:
(get_contiguous_base): Ditto.
(sve_switcher): Ditto.
* config/aarch64/aarch64.cc (all_architectures): Ditto.
(all_cores): Ditto.
(AARCH64_NO_FEATURES): Remove superceded #define and #undef.
(aarch64_override_options): Use AARCH64_NO_FEATURES.
(aarch64_process_target_attr): Remove dead initialisation.
* config/aarch64/driver-aarch64.cc
(aarch64_cpu_data): Use AARCH64_NO_FEATURES.
(aarch64_arches): Ditto.
(host_detect_local_cpu): Ditto.
diff --git a/gcc/common/config/aarch64/aarch64-common.cc b/gcc/common/config/aarch64/aarch64-common.cc
index 951d041d3109b935e90a7cb5d714940414e81761..162b622564ab543cadfc24a7341f1fc476733f45 100644
--- a/gcc/common/config/aarch64/aarch64-common.cc
+++ b/gcc/common/config/aarch64/aarch64-common.cc
@@ -158,7 +158,8 @@ static constexpr aarch64_option_extension all_extensions[] =
{NAME, AARCH64_FL_##IDENT, feature_deps::IDENT ().explicit_on, \
feature_deps::get_flags_off (feature_deps::root_off_##IDENT)},
#include "config/aarch64/aarch64-option-extensions.def"
- {NULL, 0, 0, 0}
+ {NULL, AARCH64_NO_FEATURES, AARCH64_NO_FEATURES,
+ AARCH64_NO_FEATURES}
};
struct processor_name_to_arch
@@ -183,7 +184,7 @@ static constexpr processor_name_to_arch all_cores[] =
{NAME, AARCH64_ARCH_##ARCH_IDENT, feature_deps::cpu_##CORE_IDENT},
#include "config/aarch64/aarch64-cores.def"
{"generic", AARCH64_ARCH_V8A, feature_deps::V8A ().enable},
- {"", aarch64_no_arch, 0}
+ {"", aarch64_no_arch, AARCH64_NO_FEATURES}
};
/* Map architecture revisions to their string representation. */
@@ -192,7 +193,7 @@ static constexpr arch_to_arch_name all_architectures[] =
#define AARCH64_ARCH(NAME, B, ARCH_IDENT, D, E) \
{AARCH64_ARCH_##ARCH_IDENT, NAME, feature_deps::ARCH_IDENT ().enable},
#include "config/aarch64/aarch64-arches.def"
- {aarch64_no_arch, "", 0}
+ {aarch64_no_arch, "", AARCH64_NO_FEATURES}
};
/* Parse the architecture extension string STR and update ISA_FLAGS
@@ -299,14 +300,14 @@ aarch64_get_extension_string_for_isa_flags
However, assemblers with Armv8-R AArch64 support should not have this
issue, so we don't need this fix when targeting Armv8-R. */
auto explicit_flags = (!(current_flags & AARCH64_FL_V8R)
- ? AARCH64_FL_CRC : 0);
+ ? AARCH64_FL_CRC : AARCH64_NO_FEATURES);
/* Add the features in isa_flags & ~current_flags using the smallest
possible number of extensions. We can do this by iterating over the
array in reverse order, since the array is sorted topologically.
But in order to make the output more readable, it seems better
to add the strings in definition order. */
- aarch64_feature_flags added = 0;
+ aarch64_feature_flags added = AARCH64_NO_FEATURES;
auto flags_crypto = AARCH64_FL_AES | AARCH64_FL_SHA2;
for (unsigned int i = ARRAY_SIZE (all_extensions); i-- > 0; )
{
diff --git a/gcc/config/aarch64/aarch64-feature-deps.h b/gcc/config/aarch64/aarch64-feature-deps.h
index 79126db88254b89f74a8583d50a77bc27865e265..992e133d76935d411ce4cd39480c07ea18c62ddf 100644
--- a/gcc/config/aarch64/aarch64-feature-deps.h
+++ b/gcc/config/aarch64/aarch64-feature-deps.h
@@ -26,7 +26,7 @@ namespace feature_deps {
/* Together, these definitions of get_flags take a list of
feature names (representing functions that are defined below)
and return the set of associated flags. */
-constexpr aarch64_feature_flags get_flags () { return 0; }
+constexpr aarch64_feature_flags get_flags () { return AARCH64_NO_FEATURES; }
template<typename T1, typename ...Ts>
constexpr aarch64_feature_flags
@@ -37,7 +37,7 @@ get_flags (T1 i, Ts... args)
/* Like get_flags, but return the transitive closure of those features
and the ones that they rely on. */
-constexpr aarch64_feature_flags get_enable () { return 0; }
+constexpr aarch64_feature_flags get_enable () { return AARCH64_NO_FEATURES; }
template<typename T1, typename ...Ts>
constexpr aarch64_feature_flags
@@ -97,9 +97,10 @@ template<aarch64_feature> struct info;
constexpr aarch64_feature_flags
get_flags_off (aarch64_feature_flags mask)
{
- return (0
+ return (AARCH64_NO_FEATURES
#define AARCH64_OPT_EXTENSION(A, IDENT, C, D, E, F) \
- | (feature_deps::IDENT ().enable & mask ? AARCH64_FL_##IDENT : 0)
+ | (feature_deps::IDENT ().enable & mask ? AARCH64_FL_##IDENT \
+ : AARCH64_NO_FEATURES)
#include "config/aarch64/aarch64-option-extensions.def"
);
}
diff --git a/gcc/config/aarch64/aarch64-opts.h b/gcc/config/aarch64/aarch64-opts.h
index 06a4fed3833482543891b4f7c778933f7cebd631..376d7b5ad25e8838bc83fd9ab1c6f09c6de10835 100644
--- a/gcc/config/aarch64/aarch64-opts.h
+++ b/gcc/config/aarch64/aarch64-opts.h
@@ -29,6 +29,8 @@ constexpr unsigned int AARCH64_NUM_ISA_MODES = (0
#define DEF_AARCH64_ISA_MODE(IDENT) + 1
#include "aarch64-isa-modes.def"
);
+
+#define AARCH64_NO_FEATURES aarch64_feature_flags (0)
#endif
/* The various cores that implement AArch64. */
diff --git a/gcc/config/aarch64/aarch64-protos.h b/gcc/config/aarch64/aarch64-protos.h
index 42639e9efcf1e0f9362f759ae63a31b8eeb0d581..4b1fefdd53843e97d3249bfb4d9fed2ffe60f865 100644
--- a/gcc/config/aarch64/aarch64-protos.h
+++ b/gcc/config/aarch64/aarch64-protos.h
@@ -736,7 +736,8 @@ const unsigned int AARCH64_BUILTIN_CLASS = (1 << AARCH64_BUILTIN_SHIFT) - 1;
class aarch64_simd_switcher
{
public:
- aarch64_simd_switcher (aarch64_feature_flags extra_flags = 0);
+ aarch64_simd_switcher (aarch64_feature_flags extra_flags =
+ AARCH64_NO_FEATURES);
~aarch64_simd_switcher ();
private:
diff --git a/gcc/config/aarch64/aarch64-sve-builtins-sme.def b/gcc/config/aarch64/aarch64-sve-builtins-sme.def
index 416df0b3637744503aa2f5f25fc21e204a4e2a77..4d2c96e8aa0865d5139338924f3bb4394efda04f 100644
--- a/gcc/config/aarch64/aarch64-sve-builtins-sme.def
+++ b/gcc/config/aarch64/aarch64-sve-builtins-sme.def
@@ -32,7 +32,7 @@
DEF_SME_ZA_FUNCTION_GS (NAME, SHAPE, TYPES, none, PREDS)
#endif
-#define REQUIRED_EXTENSIONS 0
+#define REQUIRED_EXTENSIONS AARCH64_NO_FEATURES
DEF_SME_FUNCTION (arm_has_sme, bool_inherent, none, none)
DEF_SME_FUNCTION (arm_in_streaming_mode, bool_inherent, none, none)
#undef REQUIRED_EXTENSIONS
diff --git a/gcc/config/aarch64/aarch64-sve-builtins.h b/gcc/config/aarch64/aarch64-sve-builtins.h
index 9cc07d5fa3debb855e787a0874a4c94bcaec65db..9028fa8554e553bfa1ea017cf3320d59ea3f82fb 100644
--- a/gcc/config/aarch64/aarch64-sve-builtins.h
+++ b/gcc/config/aarch64/aarch64-sve-builtins.h
@@ -666,7 +666,7 @@ public:
rtx convert_to_pmode (rtx);
rtx get_contiguous_base (machine_mode, unsigned int = 1, unsigned int = 2,
- aarch64_feature_flags = 0);
+ aarch64_feature_flags = AARCH64_NO_FEATURES);
rtx get_fallback_value (machine_mode, unsigned int,
unsigned int, unsigned int &);
rtx get_reg_target ();
@@ -799,7 +799,7 @@ public:
class sve_switcher : public aarch64_simd_switcher
{
public:
- sve_switcher (aarch64_feature_flags = 0);
+ sve_switcher (aarch64_feature_flags = AARCH64_NO_FEATURES);
~sve_switcher ();
private:
diff --git a/gcc/config/aarch64/aarch64-sve-builtins.cc b/gcc/config/aarch64/aarch64-sve-builtins.cc
index f3983a123e354a7626d4646363883d444fa6b8a5..d555f350cd79ebed21dab77208b0ce291ab90e79 100644
--- a/gcc/config/aarch64/aarch64-sve-builtins.cc
+++ b/gcc/config/aarch64/aarch64-sve-builtins.cc
@@ -894,7 +894,7 @@ static CONSTEXPR const function_group_info function_groups[] = {
static CONSTEXPR const function_group_info neon_sve_function_groups[] = {
#define DEF_NEON_SVE_FUNCTION(NAME, SHAPE, TYPES, GROUPS, PREDS) \
{ #NAME, &neon_sve_bridge_functions::NAME, &shapes::SHAPE, types_##TYPES, \
- groups_##GROUPS, preds_##PREDS, 0 },
+ groups_##GROUPS, preds_##PREDS, AARCH64_NO_FEATURES },
#include "aarch64-neon-sve-bridge-builtins.def"
};
diff --git a/gcc/config/aarch64/aarch64.cc b/gcc/config/aarch64/aarch64.cc
index 4e6ad1023f638c9756ee9503b1ecbd3c1573871a..582dac5129faccee0db3a68f6bdf866e8b41a059 100644
--- a/gcc/config/aarch64/aarch64.cc
+++ b/gcc/config/aarch64/aarch64.cc
@@ -450,7 +450,8 @@ static CONSTEXPR const processor all_architectures[] =
{NAME, CORE, CORE, AARCH64_ARCH_##ARCH_IDENT, \
feature_deps::ARCH_IDENT ().enable, NULL},
#include "aarch64-arches.def"
- {NULL, aarch64_none, aarch64_none, aarch64_no_arch, 0, NULL}
+ {NULL, aarch64_none, aarch64_none, aarch64_no_arch, AARCH64_NO_FEATURES,
+ NULL}
};
/* Processor cores implementing AArch64. */
@@ -460,7 +461,8 @@ static const struct processor all_cores[] =
{NAME, IDENT, SCHED, AARCH64_ARCH_##ARCH, \
feature_deps::cpu_##IDENT, &COSTS##_tunings},
#include "aarch64-cores.def"
- {NULL, aarch64_none, aarch64_none, aarch64_no_arch, 0, NULL}
+ {NULL, aarch64_none, aarch64_none, aarch64_no_arch, AARCH64_NO_FEATURES,
+ NULL}
};
/* Internal representation of system registers. */
typedef struct {
@@ -490,8 +492,6 @@ typedef struct {
#define AARCH64_FEATURES(N, ...) \
AARCH64_OR_FEATURES_##N (0, __VA_ARGS__)
-#define AARCH64_NO_FEATURES 0
-
/* Flags associated with the properties of system registers. It mainly serves
to mark particular registers as read or write only. */
#define F_DEPRECATED (1 << 1)
@@ -514,8 +514,6 @@ const sysreg_t aarch64_sysregs[] =
#undef CPENC
};
-#undef AARCH64_NO_FEATURES
-
using sysreg_map_t = hash_map<nofree_string_hash, const sysreg_t *>;
static sysreg_map_t *sysreg_map = nullptr;
@@ -18788,9 +18786,9 @@ static const struct aarch_branch_protect_type aarch64_branch_protect_types[] =
static void
aarch64_override_options (void)
{
- aarch64_feature_flags cpu_isa = 0;
- aarch64_feature_flags arch_isa = 0;
- aarch64_set_asm_isa_flags (0);
+ aarch64_feature_flags cpu_isa = AARCH64_NO_FEATURES;
+ aarch64_feature_flags arch_isa = AARCH64_NO_FEATURES;
+ aarch64_set_asm_isa_flags (AARCH64_NO_FEATURES);
const struct processor *cpu = NULL;
const struct processor *arch = NULL;
@@ -19553,7 +19551,7 @@ aarch64_process_target_attr (tree args)
{
/* Check if token is possibly an arch extension without
leading '+'. */
- aarch64_feature_flags isa_temp = 0;
+ aarch64_feature_flags isa_temp;
auto with_plus = std::string ("+") + token;
enum aarch_parse_opt_result ext_res
= aarch64_parse_extension (with_plus.c_str (), &isa_temp, nullptr);
diff --git a/gcc/config/aarch64/driver-aarch64.cc b/gcc/config/aarch64/driver-aarch64.cc
index b620351e572038d84da5d401a9c295fe19797b4e..56fd4d8d7666e569ae5f20b9d5ab8933335c4fa5 100644
--- a/gcc/config/aarch64/driver-aarch64.cc
+++ b/gcc/config/aarch64/driver-aarch64.cc
@@ -67,7 +67,8 @@ struct aarch64_core_data
static CONSTEXPR const aarch64_core_data aarch64_cpu_data[] =
{
#include "aarch64-cores.def"
- { NULL, NULL, INVALID_IMP, INVALID_CORE, ALL_VARIANTS, 0 }
+ { NULL, NULL, INVALID_IMP, INVALID_CORE, ALL_VARIANTS,
+ AARCH64_NO_FEATURES }
};
@@ -85,7 +86,7 @@ struct aarch64_arch_driver_info
static CONSTEXPR const aarch64_arch_driver_info aarch64_arches[] =
{
#include "aarch64-arches.def"
- {NULL, NULL, 0}
+ {NULL, NULL, AARCH64_NO_FEATURES}
};
@@ -261,9 +262,9 @@ host_detect_local_cpu (int argc, const char **argv)
unsigned int variants[2] = { ALL_VARIANTS, ALL_VARIANTS };
unsigned int n_variants = 0;
bool processed_exts = false;
- aarch64_feature_flags extension_flags = 0;
- aarch64_feature_flags unchecked_extension_flags = 0;
- aarch64_feature_flags default_flags = 0;
+ aarch64_feature_flags extension_flags = AARCH64_NO_FEATURES;
+ aarch64_feature_flags unchecked_extension_flags = AARCH64_NO_FEATURES;
+ aarch64_feature_flags default_flags = AARCH64_NO_FEATURES;
std::string buf;
size_t sep_pos = -1;
char *fcpu_info;
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 04/12] aarch64: Don't compare aarch64_feature_flags to 0.
2024-05-14 14:54 [PATCH 00/12] aarch64: Extend aarch64_feature_flags to 128 bits Andrew Carlotti
` (2 preceding siblings ...)
2024-05-14 14:56 ` [PATCH 03/12] aarch64: Don't use 0 for aarch64_feature_flags Andrew Carlotti
@ 2024-05-14 14:56 ` Andrew Carlotti
2024-05-14 14:56 ` [PATCH 05/12] aarch64: Eliminate a temporary variable Andrew Carlotti
` (8 subsequent siblings)
12 siblings, 0 replies; 16+ messages in thread
From: Andrew Carlotti @ 2024-05-14 14:56 UTC (permalink / raw)
To: gcc-patches; +Cc: Richard Sandiford
A later commit will disallow such comparisons. We can instead convert
directly to a boolean value, and make sure all such conversions are
explicit.
TODO: FIX SYSREG GATING.
gcc/ChangeLog:
* config/aarch64/aarch64-sve-builtins.cc
(check_required_extensions): Replace comparison with 0.
(add_overloaded_function): Ditto.
* config/aarch64/aarch64.cc (aarch64_add_offset): Ditto.
(aarch64_guard_switch_pstate_sm): Ditto.
(aarch64_switch_pstate_sm): Ditto.
(aarch64_need_old_pstate_sm): Ditto.
(aarch64_epilogue_uses): Ditto.
(aarch64_update_ipa_fn_target_info): Ditto.
(aarch64_optimize_mode_switching): Ditto.
(aarch64_mode_entry): Ditto.
(aarch64_mode_exit): Ditto.
(aarch64_valid_sysreg_name_p): Ditto.
(aarch64_retrieve_sysreg): Ditto..
* config/aarch64/aarch64.h (TARGET_STREAMING_COMPATIBLE): Ditto.
diff --git a/gcc/config/aarch64/aarch64-sve-builtins.cc b/gcc/config/aarch64/aarch64-sve-builtins.cc
index d555f350cd79ebed21dab77208b0ce291ab90e79..f033db5b25371d6b20a7c3cc2a4dc5462f8f991a 100644
--- a/gcc/config/aarch64/aarch64-sve-builtins.cc
+++ b/gcc/config/aarch64/aarch64-sve-builtins.cc
@@ -1125,7 +1125,7 @@ check_required_extensions (location_t location, tree fndecl,
aarch64_feature_flags required_extensions)
{
auto missing_extensions = required_extensions & ~aarch64_asm_isa_flags;
- if (missing_extensions == 0)
+ if (!missing_extensions)
return check_required_registers (location, fndecl);
if (missing_extensions & AARCH64_FL_SM_OFF)
@@ -1635,8 +1635,8 @@ add_overloaded_function (const function_instance &instance,
tree id = get_identifier (name);
if (registered_function **map_value = name_map->get (id))
gcc_assert ((*map_value)->instance == instance
- && ((*map_value)->required_extensions
- & ~required_extensions) == 0);
+ && !((*map_value)->required_extensions
+ & ~required_extensions));
else
{
registered_function &rfn
diff --git a/gcc/config/aarch64/aarch64.h b/gcc/config/aarch64/aarch64.h
index 8eb21cfcfc1e80bef051c571ec7cfae47e3393ed..f4ab220271239ce5a750cf211120d5b37d7f8b27 100644
--- a/gcc/config/aarch64/aarch64.h
+++ b/gcc/config/aarch64/aarch64.h
@@ -275,7 +275,7 @@ constexpr auto AARCH64_FL_DEFAULT_ISA_MODE = AARCH64_FL_SM_OFF;
/* The current function has a streaming-compatible body. */
#define TARGET_STREAMING_COMPATIBLE \
- ((aarch64_isa_flags & AARCH64_FL_SM_STATE) == 0)
+ (!(aarch64_isa_flags & AARCH64_FL_SM_STATE))
/* PSTATE.ZA is enabled in the current function body. */
#define TARGET_ZA (AARCH64_ISA_ZA_ON)
diff --git a/gcc/config/aarch64/aarch64.cc b/gcc/config/aarch64/aarch64.cc
index 582dac5129faccee0db3a68f6bdf866e8b41a059..e84151c474029b437ce67eb0cd6fca591a823b82 100644
--- a/gcc/config/aarch64/aarch64.cc
+++ b/gcc/config/aarch64/aarch64.cc
@@ -4649,7 +4649,7 @@ aarch64_add_offset (scalar_int_mode mode, rtx dest, rtx src,
{
gcc_assert (offset.coeffs[0] == offset.coeffs[1]);
rtx offset_rtx;
- if (force_isa_mode == 0)
+ if (!force_isa_mode)
offset_rtx = gen_int_mode (offset, mode);
else
offset_rtx = aarch64_sme_vq_immediate (mode, offset.coeffs[0], 0);
@@ -4675,7 +4675,7 @@ aarch64_add_offset (scalar_int_mode mode, rtx dest, rtx src,
&& aarch64_sve_addvl_addpl_immediate_p (poly_offset))
{
rtx offset_rtx;
- if (force_isa_mode == 0)
+ if (!force_isa_mode)
offset_rtx = gen_int_mode (poly_offset, mode);
else
offset_rtx = aarch64_sme_vq_immediate (mode, factor, 0);
@@ -4759,8 +4759,7 @@ aarch64_add_offset (scalar_int_mode mode, rtx dest, rtx src,
a shift and add sequence for the multiplication.
If CNTB << SHIFT is out of range, stick with the current
shift factor. */
- if (force_isa_mode == 0
- && IN_RANGE (low_bit, 2, 16 * 16))
+ if (!force_isa_mode && IN_RANGE (low_bit, 2, 16 * 16))
{
val = gen_int_mode (poly_int64 (low_bit, low_bit), mode);
shift = 0;
@@ -4900,7 +4899,7 @@ static rtx_insn *
aarch64_guard_switch_pstate_sm (rtx old_svcr, aarch64_feature_flags local_mode)
{
local_mode &= AARCH64_FL_SM_STATE;
- gcc_assert (local_mode != 0);
+ gcc_assert (local_mode);
auto already_ok_cond = (local_mode & AARCH64_FL_SM_ON ? NE : EQ);
auto *label = gen_label_rtx ();
auto branch = aarch64_gen_test_and_branch (already_ok_cond, old_svcr, 0,
@@ -4923,7 +4922,7 @@ aarch64_switch_pstate_sm (aarch64_feature_flags old_mode,
gcc_assert (old_mode != new_mode);
if ((new_mode & AARCH64_FL_SM_ON)
- || (new_mode == 0 && (old_mode & AARCH64_FL_SM_OFF)))
+ || (!new_mode && (old_mode & AARCH64_FL_SM_OFF)))
emit_insn (gen_aarch64_smstart_sm ());
else
emit_insn (gen_aarch64_smstop_sm ());
@@ -7724,7 +7723,7 @@ aarch64_need_old_pstate_sm ()
{
/* Exit early if the incoming value of PSTATE.SM is known at
compile time. */
- if (aarch64_cfun_incoming_pstate_sm () != 0)
+ if (aarch64_cfun_incoming_pstate_sm ())
return false;
if (aarch64_cfun_enables_pstate_sm ())
@@ -9407,7 +9406,7 @@ aarch64_epilogue_uses (int regno)
return 1;
/* If the function shares SME state with its caller, ensure that that
data is not in the lazy save buffer on exit. */
- if (regno == ZA_SAVED_REGNUM && aarch64_cfun_incoming_pstate_za () != 0)
+ if (regno == ZA_SAVED_REGNUM && aarch64_cfun_incoming_pstate_za ())
return 1;
if (regno == ZA_REGNUM && aarch64_cfun_shared_flags ("za") != 0)
return 1;
@@ -20631,7 +20630,7 @@ aarch64_update_ipa_fn_target_info (unsigned int &info, const gimple *stmt)
If the function isn't marked streaming-compatible then it
needs whichever SM mode it selects. */
tree decl = gimple_call_fndecl (call);
- if (aarch64_fndecl_pstate_sm (decl) != 0)
+ if (aarch64_fndecl_pstate_sm (decl))
info |= AARCH64_IPA_SM_FIXED;
}
}
@@ -29286,7 +29285,7 @@ aarch64_pars_overlap_p (rtx par1, rtx par2)
bool
aarch64_optimize_mode_switching (aarch64_mode_entity entity)
{
- bool have_sme_state = (aarch64_cfun_incoming_pstate_za () != 0
+ bool have_sme_state = (aarch64_cfun_incoming_pstate_za ()
|| (aarch64_cfun_has_new_state ("za")
&& df_regs_ever_live_p (ZA_REGNUM))
|| (aarch64_cfun_has_new_state ("zt0")
@@ -29854,7 +29853,7 @@ aarch64_mode_entry (int entity)
case aarch64_mode_entity::LOCAL_SME_STATE:
return int (aarch64_cfun_shared_flags ("za") != 0
? aarch64_local_sme_state::ACTIVE_LIVE
- : aarch64_cfun_incoming_pstate_za () != 0
+ : aarch64_cfun_incoming_pstate_za ()
? aarch64_local_sme_state::ACTIVE_DEAD
: aarch64_local_sme_state::INACTIVE_CALLER);
}
@@ -29874,7 +29873,7 @@ aarch64_mode_exit (int entity)
case aarch64_mode_entity::LOCAL_SME_STATE:
return int (aarch64_cfun_shared_flags ("za") != 0
? aarch64_local_sme_state::ACTIVE_LIVE
- : aarch64_cfun_incoming_pstate_za () != 0
+ : aarch64_cfun_incoming_pstate_za ()
? aarch64_local_sme_state::ACTIVE_DEAD
: aarch64_local_sme_state::INACTIVE_CALLER);
}
@@ -30216,7 +30215,7 @@ aarch64_valid_sysreg_name_p (const char *regname)
if (sysreg == NULL)
return aarch64_is_implem_def_reg (regname);
if (sysreg->arch_reqs)
- return (aarch64_isa_flags & sysreg->arch_reqs);
+ return (bool) (aarch64_isa_flags & sysreg->arch_reqs);
return true;
}
@@ -30240,7 +30239,7 @@ aarch64_retrieve_sysreg (const char *regname, bool write_p, bool is128op)
if ((write_p && (sysreg->properties & F_REG_READ))
|| (!write_p && (sysreg->properties & F_REG_WRITE)))
return NULL;
- if ((~aarch64_isa_flags & sysreg->arch_reqs) != 0)
+ if (~aarch64_isa_flags & sysreg->arch_reqs)
return NULL;
return sysreg->encoding;
}
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 05/12] aarch64: Eliminate a temporary variable.
2024-05-14 14:54 [PATCH 00/12] aarch64: Extend aarch64_feature_flags to 128 bits Andrew Carlotti
` (3 preceding siblings ...)
2024-05-14 14:56 ` [PATCH 04/12] aarch64: Don't compare aarch64_feature_flags to 0 Andrew Carlotti
@ 2024-05-14 14:56 ` Andrew Carlotti
2024-05-14 14:57 ` [PATCH 06/12] aarch64: Introduce aarch64_isa_mode type Andrew Carlotti
` (7 subsequent siblings)
12 siblings, 0 replies; 16+ messages in thread
From: Andrew Carlotti @ 2024-05-14 14:56 UTC (permalink / raw)
To: gcc-patches; +Cc: Richard Sandiford
The name would become misleading in a later commit anyway, and I think
this is marginally more readable.
gcc/ChangeLog:
* config/aarch64/aarch64.cc
(aarch64_override_options): Remove temporary variable.
diff --git a/gcc/config/aarch64/aarch64.cc b/gcc/config/aarch64/aarch64.cc
index e84151c474029b437ce67eb0cd6fca591a823b82..7b4e625190018dc3f16ef45c6eaf8fd3af10c784 100644
--- a/gcc/config/aarch64/aarch64.cc
+++ b/gcc/config/aarch64/aarch64.cc
@@ -18817,7 +18817,6 @@ aarch64_override_options (void)
SUBTARGET_OVERRIDE_OPTIONS;
#endif
- auto isa_mode = AARCH64_FL_DEFAULT_ISA_MODE;
if (cpu && arch)
{
/* If both -mcpu and -march are specified, warn if they are not
@@ -18840,25 +18839,25 @@ aarch64_override_options (void)
}
selected_arch = arch->arch;
- aarch64_set_asm_isa_flags (arch_isa | isa_mode);
+ aarch64_set_asm_isa_flags (arch_isa | AARCH64_FL_DEFAULT_ISA_MODE);
}
else if (cpu)
{
selected_arch = cpu->arch;
- aarch64_set_asm_isa_flags (cpu_isa | isa_mode);
+ aarch64_set_asm_isa_flags (cpu_isa | AARCH64_FL_DEFAULT_ISA_MODE);
}
else if (arch)
{
cpu = &all_cores[arch->ident];
selected_arch = arch->arch;
- aarch64_set_asm_isa_flags (arch_isa | isa_mode);
+ aarch64_set_asm_isa_flags (arch_isa | AARCH64_FL_DEFAULT_ISA_MODE);
}
else
{
/* No -mcpu or -march specified, so use the default CPU. */
cpu = &all_cores[TARGET_CPU_DEFAULT];
selected_arch = cpu->arch;
- aarch64_set_asm_isa_flags (cpu->flags | isa_mode);
+ aarch64_set_asm_isa_flags (cpu->flags | AARCH64_FL_DEFAULT_ISA_MODE);
}
selected_tune = tune ? tune->ident : cpu->ident;
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 06/12] aarch64: Introduce aarch64_isa_mode type
2024-05-14 14:54 [PATCH 00/12] aarch64: Extend aarch64_feature_flags to 128 bits Andrew Carlotti
` (4 preceding siblings ...)
2024-05-14 14:56 ` [PATCH 05/12] aarch64: Eliminate a temporary variable Andrew Carlotti
@ 2024-05-14 14:57 ` Andrew Carlotti
2024-05-14 14:57 ` [PATCH 07/12] aarch64: Define aarch64_get_{asm_|}isa_flags Andrew Carlotti
` (6 subsequent siblings)
12 siblings, 0 replies; 16+ messages in thread
From: Andrew Carlotti @ 2024-05-14 14:57 UTC (permalink / raw)
To: gcc-patches; +Cc: Richard Sandiford
Currently there are many places where an aarch64_feature_flags variable
is used, but only the bottom three isa mode bits are set and read.
Using a separate data type for these value makes it more clear that
they're not expected or required to have any of their upper feature bits
set. It will also make things simpler and more efficient when we extend
aarch64_feature_flags to 128 bits.
This patch uses explicit casts whenever converting from an
aarch64_feature_flags value to an aarch64_isa_mode value. This isn't
strictly necessary, but serves to highlight the locations where an
explicit conversion will become necessary later.
gcc/ChangeLog:
* config/aarch64/aarch64-opts.h: Add aarch64_isa_mode typedef.
* config/aarch64/aarch64-protos.h
(aarch64_gen_callee_cookie): Use aarch64_isa_mode parameter.
(aarch64_sme_vq_immediate): Ditto.
* config/aarch64/aarch64.cc
(aarch64_fntype_pstate_sm): Use aarch64_isa_mode values.
(aarch64_fntype_pstate_za): Ditto.
(aarch64_fndecl_pstate_sm): Ditto.
(aarch64_fndecl_pstate_za): Ditto.
(aarch64_fndecl_isa_mode): Ditto.
(aarch64_cfun_incoming_pstate_sm): Ditto.
(aarch64_cfun_enables_pstate_sm): Ditto.
(aarch64_call_switches_pstate_sm): Ditto.
(aarch64_gen_callee_cookie): Ditto.
(aarch64_callee_isa_mode): Ditto.
(aarch64_insn_callee_abi): Ditto.
(aarch64_sme_vq_immediate): Ditto.
(aarch64_add_offset_temporaries): Ditto.
(aarch64_add_offset): Ditto.
(aarch64_add_sp): Ditto.
(aarch64_sub_sp): Ditto.
(aarch64_guard_switch_pstate_sm): Ditto.
(aarch64_switch_pstate_sm): Ditto.
(aarch64_init_cumulative_args): Ditto.
(aarch64_allocate_and_probe_stack_space): Ditto.
(aarch64_expand_prologue): Ditto.
(aarch64_expand_epilogue): Ditto.
(aarch64_start_call_args): Ditto.
(aarch64_expand_call): Ditto.
(aarch64_end_call_args): Ditto.
(aarch64_set_current_function): Ditto, with added conversions.
(aarch64_handle_attr_arch): Avoid macro with changed type.
(aarch64_handle_attr_cpu): Ditto.
(aarch64_handle_attr_isa_flags): Ditto.
(aarch64_switch_pstate_sm_for_landing_pad):
Use arch64_isa_mode values.
(aarch64_switch_pstate_sm_for_jump): Ditto.
(pass_switch_pstate_sm::gate): Ditto.
* config/aarch64/aarch64.h
(AARCH64_ISA_MODE_{SM_ON|SM_OFF|ZA_ON}): New macros.
(AARCH64_FL_SM_STATE): Mark as possibly unused.
(AARCH64_ISA_MODE_SM_STATE): New aarch64_isa_mode mask.
(AARCH64_DEFAULT_ISA_MODE): New aarch64_isa_mode value.
(AARCH64_FL_DEFAULT_ISA_MODE): Define using above value.
(AARCH64_ISA_MODE): Change type to aarch64_isa_mode.
(arm_pcs): Use aarch64_isa_mode value.
diff --git a/gcc/config/aarch64/aarch64-opts.h b/gcc/config/aarch64/aarch64-opts.h
index 376d7b5ad25e8838bc83fd9ab1c6f09c6de10835..c2d68716857b49db8f9c1393f11b3377f51fb60c 100644
--- a/gcc/config/aarch64/aarch64-opts.h
+++ b/gcc/config/aarch64/aarch64-opts.h
@@ -23,6 +23,8 @@
#define GCC_AARCH64_OPTS_H
#ifndef USED_FOR_TARGET
+typedef uint64_t aarch64_isa_mode;
+
typedef uint64_t aarch64_feature_flags;
constexpr unsigned int AARCH64_NUM_ISA_MODES = (0
diff --git a/gcc/config/aarch64/aarch64-protos.h b/gcc/config/aarch64/aarch64-protos.h
index 4b1fefdd53843e97d3249bfb4d9fed2ffe60f865..585beee44d51275545775420905e7c7b37e2ce5c 100644
--- a/gcc/config/aarch64/aarch64-protos.h
+++ b/gcc/config/aarch64/aarch64-protos.h
@@ -768,7 +768,7 @@ bool aarch64_constant_address_p (rtx);
bool aarch64_emit_approx_div (rtx, rtx, rtx);
bool aarch64_emit_approx_sqrt (rtx, rtx, bool);
tree aarch64_vector_load_decl (tree);
-rtx aarch64_gen_callee_cookie (aarch64_feature_flags, arm_pcs);
+rtx aarch64_gen_callee_cookie (aarch64_isa_mode, arm_pcs);
void aarch64_expand_call (rtx, rtx, rtx, bool);
bool aarch64_expand_cpymem_mops (rtx *, bool);
bool aarch64_expand_cpymem (rtx *, bool);
@@ -809,7 +809,7 @@ int aarch64_add_offset_temporaries (rtx);
void aarch64_split_add_offset (scalar_int_mode, rtx, rtx, rtx, rtx, rtx);
bool aarch64_rdsvl_immediate_p (const_rtx);
rtx aarch64_sme_vq_immediate (machine_mode mode, HOST_WIDE_INT,
- aarch64_feature_flags);
+ aarch64_isa_mode);
char *aarch64_output_rdsvl (const_rtx);
bool aarch64_addsvl_addspl_immediate_p (const_rtx);
char *aarch64_output_addsvl_addspl (rtx);
diff --git a/gcc/config/aarch64/aarch64.h b/gcc/config/aarch64/aarch64.h
index f4ab220271239ce5a750cf211120d5b37d7f8b27..773cc12d5a88f774ab78af8a9099312335c19513 100644
--- a/gcc/config/aarch64/aarch64.h
+++ b/gcc/config/aarch64/aarch64.h
@@ -187,7 +187,17 @@ enum class aarch64_feature : unsigned char {
#include "aarch64-arches.def"
#undef HANDLE
-constexpr auto AARCH64_FL_SM_STATE = AARCH64_FL_SM_ON | AARCH64_FL_SM_OFF;
+/* Define aarch64_isa_mode masks. */
+#define DEF_AARCH64_ISA_MODE(IDENT) \
+ constexpr auto AARCH64_ISA_MODE_##IDENT ATTRIBUTE_UNUSED \
+ = aarch64_isa_mode (1) << int (aarch64_feature::IDENT);
+#include "aarch64-isa-modes.def"
+#undef HANDLE
+
+constexpr auto AARCH64_FL_SM_STATE ATTRIBUTE_UNUSED
+ = AARCH64_FL_SM_ON | AARCH64_FL_SM_OFF;
+constexpr auto AARCH64_ISA_MODE_SM_STATE ATTRIBUTE_UNUSED
+ = AARCH64_ISA_MODE_SM_ON | AARCH64_ISA_MODE_SM_OFF;
/* The mask of all ISA modes. */
constexpr auto AARCH64_FL_ISA_MODES
@@ -195,7 +205,10 @@ constexpr auto AARCH64_FL_ISA_MODES
/* The default ISA mode, for functions with no attributes that specify
something to the contrary. */
-constexpr auto AARCH64_FL_DEFAULT_ISA_MODE = AARCH64_FL_SM_OFF;
+constexpr auto AARCH64_DEFAULT_ISA_MODE ATTRIBUTE_UNUSED
+ = AARCH64_ISA_MODE_SM_OFF;
+constexpr auto AARCH64_FL_DEFAULT_ISA_MODE ATTRIBUTE_UNUSED
+ = aarch64_feature_flags (AARCH64_DEFAULT_ISA_MODE);
#endif
@@ -208,7 +221,7 @@ constexpr auto AARCH64_FL_DEFAULT_ISA_MODE = AARCH64_FL_SM_OFF;
#define AARCH64_ISA_SM_OFF (aarch64_isa_flags & AARCH64_FL_SM_OFF)
#define AARCH64_ISA_SM_ON (aarch64_isa_flags & AARCH64_FL_SM_ON)
#define AARCH64_ISA_ZA_ON (aarch64_isa_flags & AARCH64_FL_ZA_ON)
-#define AARCH64_ISA_MODE (aarch64_isa_flags & AARCH64_FL_ISA_MODES)
+#define AARCH64_ISA_MODE (aarch64_isa_mode) (aarch64_isa_flags & AARCH64_FL_ISA_MODES)
#define AARCH64_ISA_V8A (aarch64_isa_flags & AARCH64_FL_V8A)
#define AARCH64_ISA_V8_1A (aarch64_isa_flags & AARCH64_FL_V8_1A)
#define AARCH64_ISA_CRC (aarch64_isa_flags & AARCH64_FL_CRC)
@@ -1123,7 +1136,7 @@ enum arm_pcs
typedef struct
{
enum arm_pcs pcs_variant;
- aarch64_feature_flags isa_mode;
+ aarch64_isa_mode isa_mode;
int aapcs_arg_processed; /* No need to lay out this argument again. */
int aapcs_ncrn; /* Next Core register number. */
int aapcs_nextncrn; /* Next next core register number. */
diff --git a/gcc/config/aarch64/aarch64.cc b/gcc/config/aarch64/aarch64.cc
index 7b4e625190018dc3f16ef45c6eaf8fd3af10c784..b6300fc24c0d674edbb0df8e2d10121f2d39e7d6 100644
--- a/gcc/config/aarch64/aarch64.cc
+++ b/gcc/config/aarch64/aarch64.cc
@@ -2157,17 +2157,17 @@ aarch64_fntype_abi (const_tree fntype)
/* Return the state of PSTATE.SM on entry to functions of type FNTYPE. */
-static aarch64_feature_flags
+static aarch64_isa_mode
aarch64_fntype_pstate_sm (const_tree fntype)
{
if (lookup_attribute ("arm", "streaming", TYPE_ATTRIBUTES (fntype)))
- return AARCH64_FL_SM_ON;
+ return AARCH64_ISA_MODE_SM_ON;
if (lookup_attribute ("arm", "streaming_compatible",
TYPE_ATTRIBUTES (fntype)))
return 0;
- return AARCH64_FL_SM_OFF;
+ return AARCH64_ISA_MODE_SM_OFF;
}
/* Return state flags that describe whether and how functions of type
@@ -2182,19 +2182,19 @@ aarch64_fntype_shared_flags (const_tree fntype, const char *state_name)
/* Return the state of PSTATE.ZA on entry to functions of type FNTYPE. */
-static aarch64_feature_flags
+static aarch64_isa_mode
aarch64_fntype_pstate_za (const_tree fntype)
{
if (aarch64_fntype_shared_flags (fntype, "za")
|| aarch64_fntype_shared_flags (fntype, "zt0"))
- return AARCH64_FL_ZA_ON;
+ return AARCH64_ISA_MODE_ZA_ON;
return 0;
}
/* Return the ISA mode on entry to functions of type FNTYPE. */
-static aarch64_feature_flags
+static aarch64_isa_mode
aarch64_fntype_isa_mode (const_tree fntype)
{
return (aarch64_fntype_pstate_sm (fntype)
@@ -2215,11 +2215,11 @@ aarch64_fndecl_is_locally_streaming (const_tree fndecl)
function FNDECL. This might be different from the state of
PSTATE.SM on entry. */
-static aarch64_feature_flags
+static aarch64_isa_mode
aarch64_fndecl_pstate_sm (const_tree fndecl)
{
if (aarch64_fndecl_is_locally_streaming (fndecl))
- return AARCH64_FL_SM_ON;
+ return AARCH64_ISA_MODE_SM_ON;
return aarch64_fntype_pstate_sm (TREE_TYPE (fndecl));
}
@@ -2238,12 +2238,12 @@ aarch64_fndecl_has_state (tree fndecl, const char *state_name)
/* Return the state of PSTATE.ZA when compiling the body of function FNDECL.
This might be different from the state of PSTATE.ZA on entry. */
-static aarch64_feature_flags
+static aarch64_isa_mode
aarch64_fndecl_pstate_za (const_tree fndecl)
{
if (aarch64_fndecl_has_new_state (fndecl, "za")
|| aarch64_fndecl_has_new_state (fndecl, "zt0"))
- return AARCH64_FL_ZA_ON;
+ return AARCH64_ISA_MODE_ZA_ON;
return aarch64_fntype_pstate_za (TREE_TYPE (fndecl));
}
@@ -2251,7 +2251,7 @@ aarch64_fndecl_pstate_za (const_tree fndecl)
/* Return the ISA mode that should be used to compile the body of
function FNDECL. */
-static aarch64_feature_flags
+static aarch64_isa_mode
aarch64_fndecl_isa_mode (const_tree fndecl)
{
return (aarch64_fndecl_pstate_sm (fndecl)
@@ -2262,7 +2262,7 @@ aarch64_fndecl_isa_mode (const_tree fndecl)
This might be different from the state of PSTATE.SM in the function
body. */
-static aarch64_feature_flags
+static aarch64_isa_mode
aarch64_cfun_incoming_pstate_sm ()
{
return aarch64_fntype_pstate_sm (TREE_TYPE (cfun->decl));
@@ -2272,7 +2272,7 @@ aarch64_cfun_incoming_pstate_sm ()
This might be different from the state of PSTATE.ZA in the function
body. */
-static aarch64_feature_flags
+static aarch64_isa_mode
aarch64_cfun_incoming_pstate_za ()
{
return aarch64_fntype_pstate_za (TREE_TYPE (cfun->decl));
@@ -2304,7 +2304,7 @@ static bool
aarch64_cfun_enables_pstate_sm ()
{
return (aarch64_fndecl_is_locally_streaming (cfun->decl)
- && aarch64_cfun_incoming_pstate_sm () != AARCH64_FL_SM_ON);
+ && aarch64_cfun_incoming_pstate_sm () != AARCH64_ISA_MODE_SM_ON);
}
/* Return true if the current function has state STATE_NAME, either by
@@ -2321,9 +2321,9 @@ aarch64_cfun_has_state (const char *state_name)
the BL instruction. */
static bool
-aarch64_call_switches_pstate_sm (aarch64_feature_flags callee_mode)
+aarch64_call_switches_pstate_sm (aarch64_isa_mode callee_mode)
{
- return (callee_mode & ~AARCH64_ISA_MODE & AARCH64_FL_SM_STATE) != 0;
+ return (bool) (callee_mode & ~AARCH64_ISA_MODE & AARCH64_ISA_MODE_SM_STATE);
}
/* Implement TARGET_COMPATIBLE_VECTOR_TYPES_P. */
@@ -2392,7 +2392,7 @@ aarch64_reg_save_mode (unsigned int regno)
return the CONST_INT that should be placed in an UNSPEC_CALLEE_ABI rtx. */
rtx
-aarch64_gen_callee_cookie (aarch64_feature_flags isa_mode, arm_pcs pcs_variant)
+aarch64_gen_callee_cookie (aarch64_isa_mode isa_mode, arm_pcs pcs_variant)
{
return gen_int_mode ((unsigned int) isa_mode
| (unsigned int) pcs_variant << AARCH64_NUM_ISA_MODES,
@@ -2412,10 +2412,10 @@ aarch64_callee_abi (rtx cookie)
required ISA mode on entry to the callee, which is also the ISA
mode on return from the callee. */
-static aarch64_feature_flags
+static aarch64_isa_mode
aarch64_callee_isa_mode (rtx cookie)
{
- return UINTVAL (cookie) & AARCH64_FL_ISA_MODES;
+ return UINTVAL (cookie) & ((1 << AARCH64_NUM_ISA_MODES) - 1);
}
/* INSN is a call instruction. Return the CONST_INT stored in its
@@ -2443,7 +2443,7 @@ aarch64_insn_callee_abi (const rtx_insn *insn)
/* INSN is a call instruction. Return the required ISA mode on entry to
the callee, which is also the ISA mode on return from the callee. */
-static aarch64_feature_flags
+static aarch64_isa_mode
aarch64_insn_callee_isa_mode (const rtx_insn *insn)
{
return aarch64_callee_isa_mode (aarch64_insn_callee_cookie (insn));
@@ -3968,10 +3968,10 @@ aarch64_output_sve_vector_inc_dec (const char *operands, rtx x)
rtx
aarch64_sme_vq_immediate (machine_mode mode, HOST_WIDE_INT factor,
- aarch64_feature_flags isa_mode)
+ aarch64_isa_mode isa_mode)
{
gcc_assert (aarch64_sve_rdvl_addvl_factor_p (factor));
- if (isa_mode & AARCH64_FL_SM_ON)
+ if (isa_mode & AARCH64_ISA_MODE_SM_ON)
/* We're in streaming mode, so we can use normal poly-int values. */
return gen_int_mode ({ factor, factor }, mode);
@@ -4622,7 +4622,7 @@ aarch64_add_offset_temporaries (rtx x)
TEMP2, if nonnull, is a second temporary register that doesn't
overlap either DEST or REG.
- FORCE_ISA_MODE is AARCH64_FL_SM_ON if any variable component of OFFSET
+ FORCE_ISA_MODE is AARCH64_ISA_MODE_SM_ON if any variable component of OFFSET
is measured relative to the SME vector length instead of the current
prevailing vector length. It is 0 otherwise.
@@ -4634,7 +4634,7 @@ aarch64_add_offset_temporaries (rtx x)
static void
aarch64_add_offset (scalar_int_mode mode, rtx dest, rtx src,
poly_int64 offset, rtx temp1, rtx temp2,
- aarch64_feature_flags force_isa_mode,
+ aarch64_isa_mode force_isa_mode,
bool frame_related_p, bool emit_move_imm = true)
{
gcc_assert (emit_move_imm || temp1 != NULL_RTX);
@@ -4655,7 +4655,7 @@ aarch64_add_offset (scalar_int_mode mode, rtx dest, rtx src,
offset_rtx = aarch64_sme_vq_immediate (mode, offset.coeffs[0], 0);
rtx_insn *insn = emit_insn (gen_add3_insn (dest, src, offset_rtx));
RTX_FRAME_RELATED_P (insn) = frame_related_p;
- if (frame_related_p && (force_isa_mode & AARCH64_FL_SM_ON))
+ if (frame_related_p && (force_isa_mode & AARCH64_ISA_MODE_SM_ON))
add_reg_note (insn, REG_CFA_ADJUST_CFA,
gen_rtx_SET (dest, plus_constant (Pmode, src,
offset)));
@@ -4683,7 +4683,7 @@ aarch64_add_offset (scalar_int_mode mode, rtx dest, rtx src,
{
rtx_insn *insn = emit_insn (gen_add3_insn (dest, src, offset_rtx));
RTX_FRAME_RELATED_P (insn) = true;
- if (force_isa_mode & AARCH64_FL_SM_ON)
+ if (force_isa_mode & AARCH64_ISA_MODE_SM_ON)
add_reg_note (insn, REG_CFA_ADJUST_CFA,
gen_rtx_SET (dest, plus_constant (Pmode, src,
poly_offset)));
@@ -4717,7 +4717,7 @@ aarch64_add_offset (scalar_int_mode mode, rtx dest, rtx src,
rtx val;
if (IN_RANGE (rel_factor, -32, 31))
{
- if (force_isa_mode & AARCH64_FL_SM_ON)
+ if (force_isa_mode & AARCH64_ISA_MODE_SM_ON)
{
/* Try to use an unshifted RDSVL, otherwise fall back on
a shifted RDSVL #1. */
@@ -4764,7 +4764,7 @@ aarch64_add_offset (scalar_int_mode mode, rtx dest, rtx src,
val = gen_int_mode (poly_int64 (low_bit, low_bit), mode);
shift = 0;
}
- else if ((force_isa_mode & AARCH64_FL_SM_ON)
+ else if ((force_isa_mode & AARCH64_ISA_MODE_SM_ON)
&& aarch64_sve_rdvl_addvl_factor_p (low_bit))
{
val = aarch64_sme_vq_immediate (mode, low_bit, 0);
@@ -4867,7 +4867,7 @@ aarch64_split_add_offset (scalar_int_mode mode, rtx dest, rtx src,
static inline void
aarch64_add_sp (rtx temp1, rtx temp2, poly_int64 delta,
- aarch64_feature_flags force_isa_mode, bool emit_move_imm)
+ aarch64_isa_mode force_isa_mode, bool emit_move_imm)
{
aarch64_add_offset (Pmode, stack_pointer_rtx, stack_pointer_rtx, delta,
temp1, temp2, force_isa_mode, true, emit_move_imm);
@@ -4879,7 +4879,7 @@ aarch64_add_sp (rtx temp1, rtx temp2, poly_int64 delta,
static inline void
aarch64_sub_sp (rtx temp1, rtx temp2, poly_int64 delta,
- aarch64_feature_flags force_isa_mode,
+ aarch64_isa_mode force_isa_mode,
bool frame_related_p, bool emit_move_imm = true)
{
aarch64_add_offset (Pmode, stack_pointer_rtx, stack_pointer_rtx, -delta,
@@ -4896,11 +4896,11 @@ aarch64_sub_sp (rtx temp1, rtx temp2, poly_int64 delta,
matches LOCAL_MODE. Return the label that the branch jumps to. */
static rtx_insn *
-aarch64_guard_switch_pstate_sm (rtx old_svcr, aarch64_feature_flags local_mode)
+aarch64_guard_switch_pstate_sm (rtx old_svcr, aarch64_isa_mode local_mode)
{
- local_mode &= AARCH64_FL_SM_STATE;
+ local_mode &= AARCH64_ISA_MODE_SM_STATE;
gcc_assert (local_mode);
- auto already_ok_cond = (local_mode & AARCH64_FL_SM_ON ? NE : EQ);
+ auto already_ok_cond = (local_mode & AARCH64_ISA_MODE_SM_ON ? NE : EQ);
auto *label = gen_label_rtx ();
auto branch = aarch64_gen_test_and_branch (already_ok_cond, old_svcr, 0,
label);
@@ -4914,15 +4914,14 @@ aarch64_guard_switch_pstate_sm (rtx old_svcr, aarch64_feature_flags local_mode)
an SMSTOP SM. */
static void
-aarch64_switch_pstate_sm (aarch64_feature_flags old_mode,
- aarch64_feature_flags new_mode)
+aarch64_switch_pstate_sm (aarch64_isa_mode old_mode, aarch64_isa_mode new_mode)
{
- old_mode &= AARCH64_FL_SM_STATE;
- new_mode &= AARCH64_FL_SM_STATE;
+ old_mode &= AARCH64_ISA_MODE_SM_STATE;
+ new_mode &= AARCH64_ISA_MODE_SM_STATE;
gcc_assert (old_mode != new_mode);
- if ((new_mode & AARCH64_FL_SM_ON)
- || (!new_mode && (old_mode & AARCH64_FL_SM_OFF)))
+ if ((new_mode & AARCH64_ISA_MODE_SM_ON)
+ || (!new_mode && (old_mode & AARCH64_ISA_MODE_SM_OFF)))
emit_insn (gen_aarch64_smstart_sm ());
else
emit_insn (gen_aarch64_smstop_sm ());
@@ -7203,7 +7202,7 @@ aarch64_init_cumulative_args (CUMULATIVE_ARGS *pcum,
else
{
pcum->pcs_variant = ARM_PCS_AAPCS64;
- pcum->isa_mode = AARCH64_FL_DEFAULT_ISA_MODE;
+ pcum->isa_mode = AARCH64_DEFAULT_ISA_MODE;
}
pcum->aapcs_reg = NULL_RTX;
pcum->aapcs_arg_processed = false;
@@ -9138,14 +9137,14 @@ aarch64_emit_stack_tie (rtx reg)
then the signal handler doesn't know the state of the stack and can make no
assumptions about which pages have been probed.
- FORCE_ISA_MODE is AARCH64_FL_SM_ON if any variable component of POLY_SIZE
+ FORCE_ISA_MODE is AARCH64_ISA_MODE_SM_ON if any variable component of POLY_SIZE
is measured relative to the SME vector length instead of the current
prevailing vector length. It is 0 otherwise. */
static void
aarch64_allocate_and_probe_stack_space (rtx temp1, rtx temp2,
poly_int64 poly_size,
- aarch64_feature_flags force_isa_mode,
+ aarch64_isa_mode force_isa_mode,
bool frame_related_p,
bool final_adjustment_p)
{
@@ -9458,7 +9457,7 @@ aarch64_read_old_svcr (unsigned int regno)
static rtx_insn *
aarch64_guard_switch_pstate_sm (unsigned int regno,
- aarch64_feature_flags local_mode)
+ aarch64_isa_mode local_mode)
{
rtx old_svcr = aarch64_read_old_svcr (regno);
return aarch64_guard_switch_pstate_sm (old_svcr, local_mode);
@@ -9559,9 +9558,9 @@ aarch64_expand_prologue (void)
unsigned reg2 = frame.wb_push_candidate2;
bool emit_frame_chain = frame.emit_frame_chain;
rtx_insn *insn;
- aarch64_feature_flags force_isa_mode = 0;
+ aarch64_isa_mode force_isa_mode = 0;
if (aarch64_cfun_enables_pstate_sm ())
- force_isa_mode = AARCH64_FL_SM_ON;
+ force_isa_mode = AARCH64_ISA_MODE_SM_ON;
if (flag_stack_clash_protection
&& known_eq (callee_adjust, 0)
@@ -9765,7 +9764,7 @@ aarch64_expand_prologue (void)
emit_insn (gen_aarch64_read_svcr (svcr));
emit_move_insn (aarch64_old_svcr_mem (), svcr);
guard_label = aarch64_guard_switch_pstate_sm (svcr,
- aarch64_isa_flags);
+ AARCH64_ISA_MODE);
}
aarch64_sme_mode_switch_regs args_switch;
auto &args = crtl->args.info;
@@ -9826,9 +9825,9 @@ aarch64_expand_epilogue (rtx_call_insn *sibcall)
HOST_WIDE_INT guard_size
= 1 << param_stack_clash_protection_guard_size;
HOST_WIDE_INT guard_used_by_caller = STACK_CLASH_CALLER_GUARD;
- aarch64_feature_flags force_isa_mode = 0;
+ aarch64_isa_mode force_isa_mode = 0;
if (aarch64_cfun_enables_pstate_sm ())
- force_isa_mode = AARCH64_FL_SM_ON;
+ force_isa_mode = AARCH64_ISA_MODE_SM_ON;
/* We can re-use the registers when:
@@ -9858,8 +9857,10 @@ aarch64_expand_epilogue (rtx_call_insn *sibcall)
{
rtx_insn *guard_label = nullptr;
if (known_ge (cfun->machine->frame.old_svcr_offset, 0))
- guard_label = aarch64_guard_switch_pstate_sm (IP0_REGNUM,
- aarch64_isa_flags);
+ {
+ guard_label = aarch64_guard_switch_pstate_sm (IP0_REGNUM,
+ AARCH64_ISA_MODE);
+ }
aarch64_sme_mode_switch_regs return_switch;
if (sibcall)
return_switch.add_call_args (sibcall);
@@ -11140,7 +11141,7 @@ aarch64_start_call_args (cumulative_args_t ca_v)
{
CUMULATIVE_ARGS *ca = get_cumulative_args (ca_v);
- if (!TARGET_SME && (ca->isa_mode & AARCH64_FL_SM_ON))
+ if (!TARGET_SME && (ca->isa_mode & AARCH64_ISA_MODE_SM_ON))
{
error ("calling a streaming function requires the ISA extension %qs",
"sme");
@@ -11157,20 +11158,20 @@ aarch64_start_call_args (cumulative_args_t ca_v)
&& !aarch64_cfun_has_state ("zt0"))
error ("call to a function that shares %qs state from a function"
" that has no %qs state", "zt0", "zt0");
- else if (!TARGET_ZA && (ca->isa_mode & AARCH64_FL_ZA_ON))
+ else if (!TARGET_ZA && (ca->isa_mode & AARCH64_ISA_MODE_ZA_ON))
error ("call to a function that shares SME state from a function"
" that has no SME state");
/* If this is a call to a private ZA function, emit a marker to
indicate where any necessary set-up code could be inserted.
The code itself is inserted by the mode-switching pass. */
- if (TARGET_ZA && !(ca->isa_mode & AARCH64_FL_ZA_ON))
+ if (TARGET_ZA && !(ca->isa_mode & AARCH64_ISA_MODE_ZA_ON))
emit_insn (gen_aarch64_start_private_za_call ());
/* If this is a call to a shared-ZA function that doesn't share ZT0,
save and restore ZT0 around the call. */
if (aarch64_cfun_has_state ("zt0")
- && (ca->isa_mode & AARCH64_FL_ZA_ON)
+ && (ca->isa_mode & AARCH64_ISA_MODE_ZA_ON)
&& ca->shared_zt0_flags == 0)
aarch64_save_zt0 ();
}
@@ -11213,7 +11214,7 @@ aarch64_expand_call (rtx result, rtx mem, rtx cookie, bool sibcall)
auto callee_isa_mode = aarch64_callee_isa_mode (callee_abi);
if (aarch64_cfun_has_state ("za")
- && (callee_isa_mode & AARCH64_FL_ZA_ON)
+ && (callee_isa_mode & AARCH64_ISA_MODE_ZA_ON)
&& !shared_za_flags)
{
sorry ("call to a function that shares state other than %qs"
@@ -11361,7 +11362,7 @@ aarch64_expand_call (rtx result, rtx mem, rtx cookie, bool sibcall)
gen_rtx_REG (VNx16BImode, ZA_SAVED_REGNUM));
/* Keep the aarch64_start/end_private_za_call markers live. */
- if (!(callee_isa_mode & AARCH64_FL_ZA_ON))
+ if (!(callee_isa_mode & AARCH64_ISA_MODE_ZA_ON))
use_reg (&CALL_INSN_FUNCTION_USAGE (call_insn),
gen_rtx_REG (VNx16BImode, LOWERING_REGNUM));
@@ -11387,13 +11388,13 @@ aarch64_end_call_args (cumulative_args_t ca_v)
/* If this is a call to a private ZA function, emit a marker to
indicate where any necessary restoration code could be inserted.
The code itself is inserted by the mode-switching pass. */
- if (TARGET_ZA && !(ca->isa_mode & AARCH64_FL_ZA_ON))
+ if (TARGET_ZA && !(ca->isa_mode & AARCH64_ISA_MODE_ZA_ON))
emit_insn (gen_aarch64_end_private_za_call ());
/* If this is a call to a shared-ZA function that doesn't share ZT0,
save and restore ZT0 around the call. */
if (aarch64_cfun_has_state ("zt0")
- && (ca->isa_mode & AARCH64_FL_ZA_ON)
+ && (ca->isa_mode & AARCH64_ISA_MODE_ZA_ON)
&& ca->shared_zt0_flags == 0)
aarch64_restore_zt0 (false);
}
@@ -19059,7 +19060,7 @@ aarch64_set_current_function (tree fndecl)
auto new_isa_mode = (fndecl
? aarch64_fndecl_isa_mode (fndecl)
- : AARCH64_FL_DEFAULT_ISA_MODE);
+ : AARCH64_DEFAULT_ISA_MODE);
auto isa_flags = TREE_TARGET_OPTION (new_tree)->x_aarch64_isa_flags;
static bool reported_zt0_p;
@@ -19081,7 +19082,7 @@ aarch64_set_current_function (tree fndecl)
aarch64_pragma_target_parse. */
if (old_tree == new_tree
&& (!fndecl || aarch64_previous_fndecl)
- && (isa_flags & AARCH64_FL_ISA_MODES) == new_isa_mode)
+ && (aarch64_isa_mode) (isa_flags & AARCH64_FL_ISA_MODES) == new_isa_mode)
{
gcc_assert (AARCH64_ISA_MODE == new_isa_mode);
return;
@@ -19096,10 +19097,11 @@ aarch64_set_current_function (tree fndecl)
/* The ISA mode can vary based on function type attributes and
function declaration attributes. Make sure that the target
options correctly reflect these attributes. */
- if ((isa_flags & AARCH64_FL_ISA_MODES) != new_isa_mode)
+ if ((aarch64_isa_mode) (isa_flags & AARCH64_FL_ISA_MODES) != new_isa_mode)
{
auto base_flags = (aarch64_asm_isa_flags & ~AARCH64_FL_ISA_MODES);
- aarch64_set_asm_isa_flags (base_flags | new_isa_mode);
+ aarch64_set_asm_isa_flags (base_flags
+ | (aarch64_feature_flags) new_isa_mode);
aarch64_override_options_internal (&global_options);
new_tree = build_target_option_node (&global_options,
@@ -19164,7 +19166,8 @@ aarch64_handle_attr_arch (const char *str)
{
gcc_assert (tmp_arch);
selected_arch = tmp_arch->arch;
- aarch64_set_asm_isa_flags (tmp_flags | AARCH64_ISA_MODE);
+ aarch64_set_asm_isa_flags (tmp_flags | (aarch64_asm_isa_flags
+ & AARCH64_FL_ISA_MODES));
return true;
}
@@ -19205,7 +19208,8 @@ aarch64_handle_attr_cpu (const char *str)
gcc_assert (tmp_cpu);
selected_tune = tmp_cpu->ident;
selected_arch = tmp_cpu->arch;
- aarch64_set_asm_isa_flags (tmp_flags | AARCH64_ISA_MODE);
+ aarch64_set_asm_isa_flags (tmp_flags | (aarch64_asm_isa_flags
+ & AARCH64_FL_ISA_MODES));
return true;
}
@@ -19283,7 +19287,7 @@ aarch64_handle_attr_isa_flags (char *str)
features if the user wants to handpick specific features. */
if (strncmp ("+nothing", str, 8) == 0)
{
- isa_flags = AARCH64_ISA_MODE;
+ isa_flags &= AARCH64_FL_ISA_MODES;
str += 8;
}
@@ -29970,11 +29974,11 @@ aarch64_switch_pstate_sm_for_landing_pad (basic_block bb)
rtx_insn *guard_label = nullptr;
if (TARGET_STREAMING_COMPATIBLE)
guard_label = aarch64_guard_switch_pstate_sm (IP0_REGNUM,
- AARCH64_FL_SM_OFF);
+ AARCH64_ISA_MODE_SM_OFF);
aarch64_sme_mode_switch_regs args_switch;
args_switch.add_call_preserved_regs (df_get_live_in (bb));
args_switch.emit_prologue ();
- aarch64_switch_pstate_sm (AARCH64_FL_SM_OFF, AARCH64_FL_SM_ON);
+ aarch64_switch_pstate_sm (AARCH64_ISA_MODE_SM_OFF, AARCH64_ISA_MODE_SM_ON);
args_switch.emit_epilogue ();
if (guard_label)
emit_label (guard_label);
@@ -29998,8 +30002,8 @@ aarch64_switch_pstate_sm_for_jump (rtx_insn *jump)
rtx_insn *guard_label = nullptr;
if (TARGET_STREAMING_COMPATIBLE)
guard_label = aarch64_guard_switch_pstate_sm (IP0_REGNUM,
- AARCH64_FL_SM_OFF);
- aarch64_switch_pstate_sm (AARCH64_FL_SM_ON, AARCH64_FL_SM_OFF);
+ AARCH64_ISA_MODE_SM_OFF);
+ aarch64_switch_pstate_sm (AARCH64_ISA_MODE_SM_ON, AARCH64_ISA_MODE_SM_OFF);
if (guard_label)
emit_label (guard_label);
auto seq = get_insns ();
@@ -30094,7 +30098,7 @@ public:
bool
pass_switch_pstate_sm::gate (function *fn)
{
- return (aarch64_fndecl_pstate_sm (fn->decl) != AARCH64_FL_SM_OFF
+ return (aarch64_fndecl_pstate_sm (fn->decl) != AARCH64_ISA_MODE_SM_OFF
|| cfun->machine->call_switches_pstate_sm);
}
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 07/12] aarch64: Define aarch64_get_{asm_|}isa_flags
2024-05-14 14:54 [PATCH 00/12] aarch64: Extend aarch64_feature_flags to 128 bits Andrew Carlotti
` (5 preceding siblings ...)
2024-05-14 14:57 ` [PATCH 06/12] aarch64: Introduce aarch64_isa_mode type Andrew Carlotti
@ 2024-05-14 14:57 ` Andrew Carlotti
2024-05-14 14:58 ` [PATCH 08/12] aarch64: Decouple feature flag option storage type Andrew Carlotti
` (5 subsequent siblings)
12 siblings, 0 replies; 16+ messages in thread
From: Andrew Carlotti @ 2024-05-14 14:57 UTC (permalink / raw)
To: gcc-patches; +Cc: Richard Sandiford
Building an aarch64_feature_flags value from data within a gcc_options
or cl_target_option struct will get more complicated in a later commit.
Use a macro to avoid doing this manually in more than one location.
gcc/ChangeLog:
* common/config/aarch64/aarch64-common.cc
(aarch64_handle_option): Use new macro.
* config/aarch64/aarch64.cc
(aarch64_override_options_internal): Ditto.
(aarch64_option_print): Ditto.
(aarch64_set_current_function): Ditto.
(aarch64_can_inline_p): Ditto.
(aarch64_declare_function_name): Ditto.
(aarch64_start_file): Ditto.
* config/aarch64/aarch64.h (aarch64_get_asm_isa_flags): New
(aarch64_get_isa_flags): New.
(aarch64_asm_isa_flags): Use new macro.
(aarch64_isa_flags): Ditto.
diff --git a/gcc/common/config/aarch64/aarch64-common.cc b/gcc/common/config/aarch64/aarch64-common.cc
index 162b622564ab543cadfc24a7341f1fc476733f45..e08a0fc86590b35a595a305599dfb919f83d6906 100644
--- a/gcc/common/config/aarch64/aarch64-common.cc
+++ b/gcc/common/config/aarch64/aarch64-common.cc
@@ -111,7 +111,7 @@ aarch64_handle_option (struct gcc_options *opts,
case OPT_mgeneral_regs_only:
opts->x_target_flags |= MASK_GENERAL_REGS_ONLY;
- aarch64_set_asm_isa_flags (opts, opts->x_aarch64_asm_isa_flags);
+ aarch64_set_asm_isa_flags (opts, aarch64_get_asm_isa_flags (opts));
return true;
case OPT_mfix_cortex_a53_835769:
diff --git a/gcc/config/aarch64/aarch64.h b/gcc/config/aarch64/aarch64.h
index 773cc12d5a88f774ab78af8a9099312335c19513..49bdc7565cd5ca80fbe2d4abf30aae12841c340f 100644
--- a/gcc/config/aarch64/aarch64.h
+++ b/gcc/config/aarch64/aarch64.h
@@ -22,15 +22,18 @@
#ifndef GCC_AARCH64_H
#define GCC_AARCH64_H
+#define aarch64_get_asm_isa_flags(opts) \
+ (aarch64_feature_flags ((opts)->x_aarch64_asm_isa_flags))
+#define aarch64_get_isa_flags(opts) \
+ (aarch64_feature_flags ((opts)->x_aarch64_isa_flags))
+
/* Make these flags read-only so that all uses go via
aarch64_set_asm_isa_flags. */
#ifndef GENERATOR_FILE
#undef aarch64_asm_isa_flags
-#define aarch64_asm_isa_flags \
- ((aarch64_feature_flags) global_options.x_aarch64_asm_isa_flags)
+#define aarch64_asm_isa_flags (aarch64_get_asm_isa_flags (&global_options))
#undef aarch64_isa_flags
-#define aarch64_isa_flags \
- ((aarch64_feature_flags) global_options.x_aarch64_isa_flags)
+#define aarch64_isa_flags (aarch64_get_isa_flags (&global_options))
#endif
/* Target CPU builtins. */
diff --git a/gcc/config/aarch64/aarch64.cc b/gcc/config/aarch64/aarch64.cc
index b6300fc24c0d674edbb0df8e2d10121f2d39e7d6..eef0905069232bacc59d574cad0f6edbaf062387 100644
--- a/gcc/config/aarch64/aarch64.cc
+++ b/gcc/config/aarch64/aarch64.cc
@@ -18292,10 +18292,11 @@ aarch64_override_options_internal (struct gcc_options *opts)
&& !fixed_regs[R18_REGNUM])
error ("%<-fsanitize=shadow-call-stack%> requires %<-ffixed-x18%>");
- if ((opts->x_aarch64_isa_flags & (AARCH64_FL_SM_ON | AARCH64_FL_ZA_ON))
- && !(opts->x_aarch64_isa_flags & AARCH64_FL_SME))
+ aarch64_feature_flags isa_flags = aarch64_get_isa_flags (opts);
+ if ((isa_flags & (AARCH64_FL_SM_ON | AARCH64_FL_ZA_ON))
+ && !(isa_flags & AARCH64_FL_SME))
{
- if (opts->x_aarch64_isa_flags & AARCH64_FL_SM_ON)
+ if (isa_flags & AARCH64_FL_SM_ON)
error ("streaming functions require the ISA extension %qs", "sme");
else
error ("functions with SME state require the ISA extension %qs",
@@ -18304,8 +18305,7 @@ aarch64_override_options_internal (struct gcc_options *opts)
" option %<-march%>, or by using the %<target%>"
" attribute or pragma", "sme");
opts->x_target_flags &= ~MASK_GENERAL_REGS_ONLY;
- auto new_flags = (opts->x_aarch64_asm_isa_flags
- | feature_deps::SME ().enable);
+ auto new_flags = isa_flags | feature_deps::SME ().enable;
aarch64_set_asm_isa_flags (opts, new_flags);
}
@@ -18999,9 +18999,9 @@ aarch64_option_print (FILE *file, int indent, struct cl_target_option *ptr)
const struct processor *cpu
= aarch64_get_tune_cpu (ptr->x_selected_tune);
const struct processor *arch = aarch64_get_arch (ptr->x_selected_arch);
+ aarch64_feature_flags isa_flags = aarch64_get_asm_isa_flags(ptr);
std::string extension
- = aarch64_get_extension_string_for_isa_flags (ptr->x_aarch64_asm_isa_flags,
- arch->flags);
+ = aarch64_get_extension_string_for_isa_flags (isa_flags, arch->flags);
fprintf (file, "%*sselected tune = %s\n", indent, "", cpu->name);
fprintf (file, "%*sselected arch = %s%s\n", indent, "",
@@ -19061,7 +19061,7 @@ aarch64_set_current_function (tree fndecl)
auto new_isa_mode = (fndecl
? aarch64_fndecl_isa_mode (fndecl)
: AARCH64_DEFAULT_ISA_MODE);
- auto isa_flags = TREE_TARGET_OPTION (new_tree)->x_aarch64_isa_flags;
+ auto isa_flags = aarch64_get_isa_flags (TREE_TARGET_OPTION (new_tree));
static bool reported_zt0_p;
if (!reported_zt0_p
@@ -20662,16 +20662,16 @@ aarch64_can_inline_p (tree caller, tree callee)
: target_option_default_node);
/* Callee's ISA flags should be a subset of the caller's. */
- auto caller_asm_isa = (caller_opts->x_aarch64_asm_isa_flags
+ auto caller_asm_isa = (aarch64_get_asm_isa_flags (caller_opts)
& ~AARCH64_FL_ISA_MODES);
- auto callee_asm_isa = (callee_opts->x_aarch64_asm_isa_flags
+ auto callee_asm_isa = (aarch64_get_asm_isa_flags (callee_opts)
& ~AARCH64_FL_ISA_MODES);
if (callee_asm_isa & ~caller_asm_isa)
return false;
- auto caller_isa = (caller_opts->x_aarch64_isa_flags
+ auto caller_isa = (aarch64_get_isa_flags (caller_opts)
& ~AARCH64_FL_ISA_MODES);
- auto callee_isa = (callee_opts->x_aarch64_isa_flags
+ auto callee_isa = (aarch64_get_isa_flags (callee_opts)
& ~AARCH64_FL_ISA_MODES);
if (callee_isa & ~caller_isa)
return false;
@@ -20691,8 +20691,8 @@ aarch64_can_inline_p (tree caller, tree callee)
PSTATE.SM mode. Otherwise the caller and callee must agree on
PSTATE.SM mode, unless we can prove that the callee is naturally
streaming-compatible. */
- auto caller_sm = (caller_opts->x_aarch64_isa_flags & AARCH64_FL_SM_STATE);
- auto callee_sm = (callee_opts->x_aarch64_isa_flags & AARCH64_FL_SM_STATE);
+ auto caller_sm = (aarch64_get_isa_flags (caller_opts) & AARCH64_FL_SM_STATE);
+ auto callee_sm = (aarch64_get_isa_flags (callee_opts) & AARCH64_FL_SM_STATE);
if (callee_sm
&& caller_sm != callee_sm
&& callee_has_property (AARCH64_IPA_SM_FIXED))
@@ -20705,8 +20705,8 @@ aarch64_can_inline_p (tree caller, tree callee)
The only other problematic case for ZA is inlining a function that
directly clobbers ZA or ZT0 into a function that has ZA or ZT0 state. */
- auto caller_za = (caller_opts->x_aarch64_isa_flags & AARCH64_FL_ZA_ON);
- auto callee_za = (callee_opts->x_aarch64_isa_flags & AARCH64_FL_ZA_ON);
+ auto caller_za = (aarch64_get_isa_flags (caller_opts) & AARCH64_FL_ZA_ON);
+ auto callee_za = (aarch64_get_isa_flags (callee_opts) & AARCH64_FL_ZA_ON);
if (!caller_za && callee_za)
return false;
if (!callee_za
@@ -24440,7 +24440,7 @@ aarch64_declare_function_name (FILE *stream, const char* name,
const struct processor *this_arch
= aarch64_get_arch (targ_options->x_selected_arch);
- auto isa_flags = targ_options->x_aarch64_asm_isa_flags;
+ auto isa_flags = aarch64_get_asm_isa_flags (targ_options);
std::string extension
= aarch64_get_extension_string_for_isa_flags (isa_flags,
this_arch->flags);
@@ -24570,7 +24570,7 @@ aarch64_start_file (void)
const struct processor *default_arch
= aarch64_get_arch (default_options->x_selected_arch);
- auto default_isa_flags = default_options->x_aarch64_asm_isa_flags;
+ auto default_isa_flags = aarch64_get_asm_isa_flags (default_options);
std::string extension
= aarch64_get_extension_string_for_isa_flags (default_isa_flags,
default_arch->flags);
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 08/12] aarch64: Decouple feature flag option storage type
2024-05-14 14:54 [PATCH 00/12] aarch64: Extend aarch64_feature_flags to 128 bits Andrew Carlotti
` (6 preceding siblings ...)
2024-05-14 14:57 ` [PATCH 07/12] aarch64: Define aarch64_get_{asm_|}isa_flags Andrew Carlotti
@ 2024-05-14 14:58 ` Andrew Carlotti
2024-05-14 14:58 ` [PATCH 09/12] aarch64: Assign flags to local constexpr variable Andrew Carlotti
` (4 subsequent siblings)
12 siblings, 0 replies; 16+ messages in thread
From: Andrew Carlotti @ 2024-05-14 14:58 UTC (permalink / raw)
To: gcc-patches; +Cc: Richard Sandiford
The awk scripts that process the .opt files are relatively fragile and
only handle a limited set of data types correctly. The unrecognised
aarch64_feature_flags type is handled as a uint64_t, which happens to be
correct for now. However, that assumption will change when we extend
the mask to 128 bits.
This patch changes the option members to use uint64_t types, and adds a
"_0" suffix to the names (both for future extensibility, and to allow
the original name to be used for the full aarch64_feature_flags mask
within generator files).
gcc/ChangeLog:
* common/config/aarch64/aarch64-common.cc
(aarch64_set_asm_isa_flags): Reorder, and add suffix to names.
* config/aarch64/aarch64.h
(aarch64_get_asm_isa_flags): Add "_0" suffix.
(aarch64_get_isa_flags): Ditto.
(aarch64_asm_isa_flags): Redefine using renamed uint64_t value.
(aarch64_isa_flags): Ditto.
* config/aarch64/aarch64.opt:
(aarch64_asm_isa_flags): Rename to...
(aarch64_asm_isa_flags_0): ...this, and change to uint64_t.
(aarch64_isa_flags): Rename to...
(aarch64_isa_flags_0): ...this, and change to uint64_t.
diff --git a/gcc/common/config/aarch64/aarch64-common.cc b/gcc/common/config/aarch64/aarch64-common.cc
index e08a0fc86590b35a595a305599dfb919f83d6906..2f437b82a24c16d9f808a4367ce2a281a49a77ee 100644
--- a/gcc/common/config/aarch64/aarch64-common.cc
+++ b/gcc/common/config/aarch64/aarch64-common.cc
@@ -66,15 +66,16 @@ static const struct default_options aarch_option_optimization_table[] =
{ OPT_LEVELS_NONE, 0, NULL, 0 }
};
-/* Set OPTS->x_aarch64_asm_isa_flags to FLAGS and update
- OPTS->x_aarch64_isa_flags accordingly. */
+
+/* Set OPTS->x_aarch64_asm_isa_flags_0 to FLAGS and update
+ OPTS->x_aarch64_isa_flags_0 accordingly. */
void
aarch64_set_asm_isa_flags (gcc_options *opts, aarch64_feature_flags flags)
{
- opts->x_aarch64_asm_isa_flags = flags;
- opts->x_aarch64_isa_flags = flags;
+ opts->x_aarch64_asm_isa_flags_0 = flags;
if (opts->x_target_flags & MASK_GENERAL_REGS_ONLY)
- opts->x_aarch64_isa_flags &= ~feature_deps::get_flags_off (AARCH64_FL_FP);
+ flags &= ~feature_deps::get_flags_off (AARCH64_FL_FP);
+ opts->x_aarch64_isa_flags_0 = flags;
}
/* Implement TARGET_HANDLE_OPTION.
diff --git a/gcc/config/aarch64/aarch64.h b/gcc/config/aarch64/aarch64.h
index 49bdc7565cd5ca80fbe2d4abf30aae12841c340f..af256c581aedc04e4194ac0158380fcdb8b65594 100644
--- a/gcc/config/aarch64/aarch64.h
+++ b/gcc/config/aarch64/aarch64.h
@@ -23,13 +23,18 @@
#define GCC_AARCH64_H
#define aarch64_get_asm_isa_flags(opts) \
- (aarch64_feature_flags ((opts)->x_aarch64_asm_isa_flags))
+ (aarch64_feature_flags ((opts)->x_aarch64_asm_isa_flags_0))
#define aarch64_get_isa_flags(opts) \
- (aarch64_feature_flags ((opts)->x_aarch64_isa_flags))
+ (aarch64_feature_flags ((opts)->x_aarch64_isa_flags_0))
/* Make these flags read-only so that all uses go via
aarch64_set_asm_isa_flags. */
-#ifndef GENERATOR_FILE
+#ifdef GENERATOR_FILE
+#undef aarch64_asm_isa_flags
+#define aarch64_asm_isa_flags (aarch64_feature_flags (aarch64_asm_isa_flags_0))
+#undef aarch64_isa_flags
+#define aarch64_isa_flags (aarch64_feature_flags (aarch64_isa_flags_0))
+#else
#undef aarch64_asm_isa_flags
#define aarch64_asm_isa_flags (aarch64_get_asm_isa_flags (&global_options))
#undef aarch64_isa_flags
diff --git a/gcc/config/aarch64/aarch64.opt b/gcc/config/aarch64/aarch64.opt
index 6356c419399bd324929cd599e5a4b926b0383469..45aab49de27bdfa0fb3f67ec06c7dcf0ac242fb3 100644
--- a/gcc/config/aarch64/aarch64.opt
+++ b/gcc/config/aarch64/aarch64.opt
@@ -31,10 +31,10 @@ TargetVariable
enum aarch64_arch selected_arch = aarch64_no_arch
TargetVariable
-aarch64_feature_flags aarch64_asm_isa_flags = 0
+uint64_t aarch64_asm_isa_flags_0 = 0
TargetVariable
-aarch64_feature_flags aarch64_isa_flags = 0
+uint64_t aarch64_isa_flags_0 = 0
TargetVariable
unsigned aarch_enable_bti = 2
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 09/12] aarch64: Assign flags to local constexpr variable
2024-05-14 14:54 [PATCH 00/12] aarch64: Extend aarch64_feature_flags to 128 bits Andrew Carlotti
` (7 preceding siblings ...)
2024-05-14 14:58 ` [PATCH 08/12] aarch64: Decouple feature flag option storage type Andrew Carlotti
@ 2024-05-14 14:58 ` Andrew Carlotti
2024-05-14 14:59 ` [PATCH 10/12] aarch64: Add aarch64_feature_flags_from_index macro Andrew Carlotti
` (3 subsequent siblings)
12 siblings, 0 replies; 16+ messages in thread
From: Andrew Carlotti @ 2024-05-14 14:58 UTC (permalink / raw)
To: gcc-patches; +Cc: Richard Sandiford
This guarantees that the constant values are actually evaluated at
compile time.
In previous testing, I have observed GCC failing to evaluate and inline
these constant values, which exposed a separate bug in which some of the
required symbols from feature_deps were missing. Richard Sandiford has
since fixed that bug, but we still want to ensure we get the benefits of
compile-time evaluation here.
gcc/ChangeLog:
* common/config/aarch64/aarch64-common.cc
(aarch64_set_asm_isa_flags): Make constant explicitly constexpr.
* config/aarch64/aarch64.cc
(aarch64_override_options_internal): Ditto.
diff --git a/gcc/common/config/aarch64/aarch64-common.cc b/gcc/common/config/aarch64/aarch64-common.cc
index 2f437b82a24c16d9f808a4367ce2a281a49a77ee..9f583bb80456709e0028c358a1bad23ad59f20f4 100644
--- a/gcc/common/config/aarch64/aarch64-common.cc
+++ b/gcc/common/config/aarch64/aarch64-common.cc
@@ -74,7 +74,10 @@ aarch64_set_asm_isa_flags (gcc_options *opts, aarch64_feature_flags flags)
{
opts->x_aarch64_asm_isa_flags_0 = flags;
if (opts->x_target_flags & MASK_GENERAL_REGS_ONLY)
- flags &= ~feature_deps::get_flags_off (AARCH64_FL_FP);
+ {
+ constexpr auto flags_mask = ~feature_deps::get_flags_off (AARCH64_FL_FP);
+ flags &= flags_mask;
+ }
opts->x_aarch64_isa_flags_0 = flags;
}
diff --git a/gcc/config/aarch64/aarch64.cc b/gcc/config/aarch64/aarch64.cc
index eef0905069232bacc59d574cad0f6edbaf062387..69c3b257982b4a0e282cbf7486802b147d166945 100644
--- a/gcc/config/aarch64/aarch64.cc
+++ b/gcc/config/aarch64/aarch64.cc
@@ -18305,7 +18305,8 @@ aarch64_override_options_internal (struct gcc_options *opts)
" option %<-march%>, or by using the %<target%>"
" attribute or pragma", "sme");
opts->x_target_flags &= ~MASK_GENERAL_REGS_ONLY;
- auto new_flags = isa_flags | feature_deps::SME ().enable;
+ constexpr auto flags_enable_sme = feature_deps::SME ().enable;
+ auto new_flags = isa_flags | flags_enable_sme;
aarch64_set_asm_isa_flags (opts, new_flags);
}
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 10/12] aarch64: Add aarch64_feature_flags_from_index macro
2024-05-14 14:54 [PATCH 00/12] aarch64: Extend aarch64_feature_flags to 128 bits Andrew Carlotti
` (8 preceding siblings ...)
2024-05-14 14:58 ` [PATCH 09/12] aarch64: Assign flags to local constexpr variable Andrew Carlotti
@ 2024-05-14 14:59 ` Andrew Carlotti
2024-05-14 14:59 ` [RFC 11/12] Add explicit bool casts to .md condition users Andrew Carlotti
` (2 subsequent siblings)
12 siblings, 0 replies; 16+ messages in thread
From: Andrew Carlotti @ 2024-05-14 14:59 UTC (permalink / raw)
To: gcc-patches; +Cc: Richard Sandiford
When aarch64_feature_flags grows to 128 bits, constructing a mask with a
specific indexed value set will become more complicated. Extract this
operation into a separate macro, and preemptively annotate the feature
masks as possibly unused.
gcc/ChangeLog:
* config/aarch64/aarch64-opts.h
(aarch64_feature_flags_from_index): New macro.
* config/aarch64/aarch64.h
(AARCH64_FL_##IDENT): Mark as maybe unused, and use new macro.
diff --git a/gcc/config/aarch64/aarch64-opts.h b/gcc/config/aarch64/aarch64-opts.h
index c2d68716857b49db8f9c1393f11b3377f51fb60c..80926a008aa2ed7dffa79aaa425dd3d7fc9d2581 100644
--- a/gcc/config/aarch64/aarch64-opts.h
+++ b/gcc/config/aarch64/aarch64-opts.h
@@ -32,6 +32,9 @@ constexpr unsigned int AARCH64_NUM_ISA_MODES = (0
#include "aarch64-isa-modes.def"
);
+#define aarch64_feature_flags_from_index(index) \
+ (aarch64_feature_flags (uint64_t (1) << index))
+
#define AARCH64_NO_FEATURES aarch64_feature_flags (0)
#endif
diff --git a/gcc/config/aarch64/aarch64.h b/gcc/config/aarch64/aarch64.h
index af256c581aedc04e4194ac0158380fcdb8b65594..dd3437214e1597f03ac947a09c124ea0b04e27e8 100644
--- a/gcc/config/aarch64/aarch64.h
+++ b/gcc/config/aarch64/aarch64.h
@@ -185,8 +185,8 @@ enum class aarch64_feature : unsigned char {
/* Define unique flags for each of the above. */
#define HANDLE(IDENT) \
- constexpr auto AARCH64_FL_##IDENT \
- = aarch64_feature_flags (1) << int (aarch64_feature::IDENT);
+ constexpr auto AARCH64_FL_##IDENT ATTRIBUTE_UNUSED \
+ = aarch64_feature_flags_from_index (int (aarch64_feature::IDENT));
#define DEF_AARCH64_ISA_MODE(IDENT) HANDLE (IDENT)
#define AARCH64_OPT_EXTENSION(A, IDENT, C, D, E, F) HANDLE (IDENT)
#define AARCH64_ARCH(A, B, IDENT, D, E) HANDLE (IDENT)
^ permalink raw reply [flat|nested] 16+ messages in thread
* [RFC 11/12] Add explicit bool casts to .md condition users
2024-05-14 14:54 [PATCH 00/12] aarch64: Extend aarch64_feature_flags to 128 bits Andrew Carlotti
` (9 preceding siblings ...)
2024-05-14 14:59 ` [PATCH 10/12] aarch64: Add aarch64_feature_flags_from_index macro Andrew Carlotti
@ 2024-05-14 14:59 ` Andrew Carlotti
2024-05-14 15:00 ` [PATCH 12/12] aarch64: Extend aarch64_feature_flags to 128 bits Andrew Carlotti
2024-05-17 15:45 ` [PATCH 00/12] " Richard Sandiford
12 siblings, 0 replies; 16+ messages in thread
From: Andrew Carlotti @ 2024-05-14 14:59 UTC (permalink / raw)
To: gcc-patches; +Cc: Richard Sandiford
This patch is one way to fix some issues I discovered when disallowing
implicit casts to bool from aarch64_feature_flags (in a later patch).
That in turn was necessary to prohibit accidental conversion of an
aarch64_feature_flags value to an integer by first implicitly casting to
a bool (and thus setting the resulting integer value to 0 or 1).
Most of the uses of TARGET_ macros occur indirectly in middle end code,
via their use in instruction pattern conditions. There are also a few
uses in aarch64 backend code, which are also changed in this patch.
The documentation on instruction patterns [1] doesn't explicitly say
that the condition must be a bool. If we want to assume this, I think
we should update the documentation, and ideally enforce type consistency
within the compiler.
The code generated in genconditions.cc by write_one_condition already
includes an assumption that casting a condition's value to an int is
valid (i.e. that it does not invoke undefined behaviour, and does not
change the result obtained when later converting it to a boolean
result). Fortunately, for aarch64 at least, this assumption only needs
to hold when the original constant is a compile time constant, whereas
all our problematic usage involves comparisons against the runtime
feature mask.
If the use of non-bool instruction pattern conditions should be
disallowed, then it would be straightforward to fix the type mismatches
in the aarch64 backend, by adding explicit bool casts to all of the
TARGET_* macros. Indeed, I think that would be a better approach to
fixing this issue. However, I felt it would be more useful to first
investigate and demonstrate the downstream impact of these type issues.
Note that this patch doesn't compile without the subsequent patch,
due to ambiguous calls to aarch64_def_or_undef(int, ...). I expect to
replace this patch with one that avoids the issue, so it isn't worth
meddling with the next patch in the series just to make this RFC compile
by itself.
[1] https://gcc.gnu.org/onlinedocs/gccint/Patterns.html
diff --git a/gcc/c-family/c-cppbuiltin.cc b/gcc/c-family/c-cppbuiltin.cc
index b6f25e4db3c06a1addc09a47335fe5184cb4a100..0cfcac6ba6b1e0ae7cdc0fb864eb28ec7de78605 100644
--- a/gcc/c-family/c-cppbuiltin.cc
+++ b/gcc/c-family/c-cppbuiltin.cc
@@ -1506,7 +1506,7 @@ c_cpp_builtins (cpp_reader *pfile)
#ifdef HAVE_adddf3
builtin_define_with_int_value ("__LIBGCC_HAVE_HWDBL__",
- HAVE_adddf3);
+ (bool) HAVE_adddf3);
#endif
}
diff --git a/gcc/config/aarch64/aarch64-c.cc b/gcc/config/aarch64/aarch64-c.cc
index fe1a20e4e546a68e5f7eddff3bbb0d3e831fbd9b..de4b383cda92c160bd706f9085999daac5d8313a 100644
--- a/gcc/config/aarch64/aarch64-c.cc
+++ b/gcc/config/aarch64/aarch64-c.cc
@@ -47,6 +47,12 @@ aarch64_def_or_undef (bool def_p, const char *macro, cpp_reader *pfile)
cpp_undef (pfile, macro);
}
+static void
+aarch64_def_or_undef (aarch64_feature_flags def_p, const char *macro, cpp_reader *pfile)
+{
+ aarch64_def_or_undef ((bool) def_p, macro, pfile);
+}
+
/* Define the macros that we always expect to have on AArch64. */
static void
diff --git a/gcc/config/aarch64/aarch64.cc b/gcc/config/aarch64/aarch64.cc
index 69c3b257982b4a0e282cbf7486802b147d166945..052cf297e7672abf015a085ab357836cb3b235e4 100644
--- a/gcc/config/aarch64/aarch64.cc
+++ b/gcc/config/aarch64/aarch64.cc
@@ -6561,10 +6561,10 @@ aarch64_function_value_regno_p (const unsigned int regno)
/* Up to four fp/simd registers can return a function value, e.g. a
homogeneous floating-point aggregate having four members. */
if (regno >= V0_REGNUM && regno < V0_REGNUM + HA_MAX_NUM_FLDS)
- return TARGET_FLOAT;
+ return (bool) TARGET_FLOAT;
if (regno >= P0_REGNUM && regno < P0_REGNUM + HA_MAX_NUM_FLDS)
- return TARGET_SVE;
+ return (bool) TARGET_SVE;
return false;
}
diff --git a/gcc/genconditions.cc b/gcc/genconditions.cc
index 13963dc3ff46aa250c39ce80d0b92356390e41ff..3aee4428ff7ff5c97260f56a5f6b0fffa4e95fc2 100644
--- a/gcc/genconditions.cc
+++ b/gcc/genconditions.cc
@@ -140,9 +140,9 @@ write_one_condition (void **slot, void * ARG_UNUSED (dummy))
putchar (*p);
}
- fputs ("\",\n __builtin_constant_p ", stdout);
+ fputs ("\",\n __builtin_constant_p ((bool)", stdout);
rtx_reader_ptr->print_c_condition (test->expr);
- fputs ("\n ? (int) ", stdout);
+ fputs (")\n ? (int) (bool)", stdout);
rtx_reader_ptr->print_c_condition (test->expr);
fputs ("\n : -1 },\n", stdout);
return 1;
diff --git a/gcc/genopinit.cc b/gcc/genopinit.cc
index d8682b2a9ad56a0a62b4407741c695489c72795b..0d9cf0de8b93da5884a352858b343f81644f9d3f 100644
--- a/gcc/genopinit.cc
+++ b/gcc/genopinit.cc
@@ -386,7 +386,7 @@ main (int argc, const char **argv)
unsigned end = MIN (patterns.length (),
(i + 1) * patterns_per_function);
for (j = start; j < end; ++j)
- fprintf (s_file, " ena[%u] = HAVE_%s;\n", j, patterns[j].name);
+ fprintf (s_file, " ena[%u] = (bool) HAVE_%s;\n", j, patterns[j].name);
fprintf (s_file, "}\n\n");
}
@@ -402,7 +402,7 @@ main (int argc, const char **argv)
"(struct target_optabs *optabs)\n{\n");
fprintf (s_file, " bool *ena = optabs->pat_enable;\n");
for (i = 0; patterns.iterate (i, &p); ++i)
- fprintf (s_file, " ena[%u] = HAVE_%s;\n", i, p->name);
+ fprintf (s_file, " ena[%u] = (bool) HAVE_%s;\n", i, p->name);
fprintf (s_file, "}\n\n");
}
diff --git a/gcc/targhooks.cc b/gcc/targhooks.cc
index 8c84deea97d31a844d0c217004d13cc5e95e744d..97117c67e736211d86a231ac046f51a5e9459c43 100644
--- a/gcc/targhooks.cc
+++ b/gcc/targhooks.cc
@@ -620,7 +620,7 @@ bool
default_float_exceptions_rounding_supported_p (void)
{
#ifdef HAVE_adddf3
- return HAVE_adddf3;
+ return (bool) HAVE_adddf3;
#else
return false;
#endif
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH 12/12] aarch64: Extend aarch64_feature_flags to 128 bits
2024-05-14 14:54 [PATCH 00/12] aarch64: Extend aarch64_feature_flags to 128 bits Andrew Carlotti
` (10 preceding siblings ...)
2024-05-14 14:59 ` [RFC 11/12] Add explicit bool casts to .md condition users Andrew Carlotti
@ 2024-05-14 15:00 ` Andrew Carlotti
2024-05-17 15:45 ` [PATCH 00/12] " Richard Sandiford
12 siblings, 0 replies; 16+ messages in thread
From: Andrew Carlotti @ 2024-05-14 15:00 UTC (permalink / raw)
To: gcc-patches; +Cc: Richard Sandiford
Replace the existing typedef with a new class containing two private
uint64_t members.
Most of the preparatory work was carried out in previous commits. The
most notable remaining changes are the addition of the get_isa_mode and
with_isa_mode functions for conversion to or from aarch64_isa_mode
types, and the use of a 'save' member function from within
aarch64_set_asm_isa_flags, to avoid needing to expose the uint64_t
members.
gcc/ChangeLog:
* common/config/aarch64/aarch64-common.cc
(aarch64_set_asm_isa_flags): Use new flags.save function.
* config/aarch64/aarch64-opts.h
(class aarch64_feature_flags): New class.
(aarch64_feature_flags_from_index): Update to handle 128 bits.
(AARCH64_NO_FEATURES): Pass a second constructor parameter.
* config/aarch64/aarch64.cc
(aarch64_guard_switch_pstate_sm): Extract isa mode explicitly.
(aarch64_expand_epilogue): Ditto.
(aarch64_expand_call): Ditto
(aarch64_set_current_function): Set/extract isa mode explicitly.
* config/aarch64/aarch64.h
(aarch64_get_asm_isa_flags): Use new option struct member.
(aarch64_get_isa_flags): Use new option struct member.
(aarch64_asm_isa_flags): Use second global variable.
(aarch64_isa_flags): Ditto.
(AARCH64_FL_ISA_MODES): Pass a second constructor parameter.
(AARCH64_FL_DEFAULT_ISA_MODE): Ditto.
(AARCH64_ISA_MODE): Extract isa mode explicitly.
* config/aarch64/aarch64.opt
(aarch64_asm_isa_flags_1): Add a second uint64_t for bitmask.
(aarch64_isa_flags_1): Ditto.
diff --git a/gcc/common/config/aarch64/aarch64-common.cc b/gcc/common/config/aarch64/aarch64-common.cc
index 9f583bb80456709e0028c358a1bad23ad59f20f4..a84650086ba9a1054f3ba15022567a00b7fb4313 100644
--- a/gcc/common/config/aarch64/aarch64-common.cc
+++ b/gcc/common/config/aarch64/aarch64-common.cc
@@ -67,18 +67,18 @@ static const struct default_options aarch_option_optimization_table[] =
};
-/* Set OPTS->x_aarch64_asm_isa_flags_0 to FLAGS and update
- OPTS->x_aarch64_isa_flags_0 accordingly. */
+/* Set OPTS->x_aarch64_asm_isa_flags_<0..n> to FLAGS and update
+ OPTS->x_aarch64_isa_flags_<0..n> accordingly. */
void
aarch64_set_asm_isa_flags (gcc_options *opts, aarch64_feature_flags flags)
{
- opts->x_aarch64_asm_isa_flags_0 = flags;
+ flags.save(&opts->x_aarch64_asm_isa_flags_0, &opts->x_aarch64_asm_isa_flags_1);
if (opts->x_target_flags & MASK_GENERAL_REGS_ONLY)
{
constexpr auto flags_mask = ~feature_deps::get_flags_off (AARCH64_FL_FP);
flags &= flags_mask;
}
- opts->x_aarch64_isa_flags_0 = flags;
+ flags.save(&opts->x_aarch64_isa_flags_0, &opts->x_aarch64_isa_flags_1);
}
/* Implement TARGET_HANDLE_OPTION.
diff --git a/gcc/config/aarch64/aarch64-opts.h b/gcc/config/aarch64/aarch64-opts.h
index 80926a008aa2ed7dffa79aaa425dd3d7fc9d2581..7571385740d5271ab99bcc3380899a550788592d 100644
--- a/gcc/config/aarch64/aarch64-opts.h
+++ b/gcc/config/aarch64/aarch64-opts.h
@@ -25,17 +25,110 @@
#ifndef USED_FOR_TARGET
typedef uint64_t aarch64_isa_mode;
-typedef uint64_t aarch64_feature_flags;
-
constexpr unsigned int AARCH64_NUM_ISA_MODES = (0
#define DEF_AARCH64_ISA_MODE(IDENT) + 1
#include "aarch64-isa-modes.def"
);
+class aarch64_feature_flags
+{
+private:
+ uint64_t flags0;
+ uint64_t flags1;
+
+public:
+ constexpr aarch64_feature_flags (uint64_t flags0_m, uint64_t flags1_m)
+ : flags0 (flags0_m), flags1 (flags1_m) {}
+ aarch64_feature_flags () = default;
+
+ void save(uint64_t *save0, uint64_t *save1)
+ {
+ *save0 = flags0;
+ *save1 = flags1;
+ }
+
+ constexpr aarch64_isa_mode get_isa_mode ()
+ {
+ return flags0 & ((1 << AARCH64_NUM_ISA_MODES) - 1);
+ }
+
+ constexpr aarch64_feature_flags with_isa_mode (const aarch64_isa_mode mode) const
+ {
+ return aarch64_feature_flags ((flags0 & ~((1 << AARCH64_NUM_ISA_MODES) - 1)) | mode,
+ flags1);
+ }
+
+ constexpr aarch64_feature_flags operator&(const aarch64_feature_flags other) const
+ {
+ return aarch64_feature_flags (flags0 & other.flags0,
+ flags1 & other.flags1);
+ }
+
+ aarch64_feature_flags operator&=(const aarch64_feature_flags other)
+ {
+ flags0 &= other.flags0;
+ flags1 &= other.flags1;
+ return *this;
+ }
+
+ constexpr aarch64_feature_flags operator|(const aarch64_feature_flags other) const
+ {
+ return aarch64_feature_flags (flags0 | other.flags0,
+ flags1 | other.flags1);
+ }
+
+ aarch64_feature_flags operator|=(const aarch64_feature_flags other)
+ {
+ flags0 |= other.flags0;
+ flags1 |= other.flags1;
+ return *this;
+ }
+
+ constexpr aarch64_feature_flags operator^(const aarch64_feature_flags other) const
+ {
+ return aarch64_feature_flags (flags0 ^ other.flags0,
+ flags1 ^ other.flags1);
+ }
+
+ aarch64_feature_flags operator^=(const aarch64_feature_flags other)
+ {
+ flags0 ^= other.flags0;
+ flags1 ^= other.flags1;
+ return *this;
+ }
+
+ constexpr aarch64_feature_flags operator~() const
+ {
+ return aarch64_feature_flags (~flags0, ~flags1);
+ }
+
+ constexpr bool operator!() const
+ {
+ return !flags0 && !flags1;
+ }
+
+ constexpr explicit operator bool() const
+ {
+ return ((bool) flags0) || ((bool) flags1);
+ }
+
+ constexpr bool operator==(const aarch64_feature_flags other) const
+ {
+ return flags0 == other.flags0 && flags1 == other.flags1;
+ }
+
+ constexpr bool operator!=(const aarch64_feature_flags other) const
+ {
+ return flags0 != other.flags0 || flags1 != other.flags1;
+ }
+
+};
+
#define aarch64_feature_flags_from_index(index) \
- (aarch64_feature_flags (uint64_t (1) << index))
+ (aarch64_feature_flags ((index < 64) ? uint64_t (1) << index : 0, \
+ (index >= 64) ? uint64_t (1) << (index - 64) : 0))
-#define AARCH64_NO_FEATURES aarch64_feature_flags (0)
+#define AARCH64_NO_FEATURES aarch64_feature_flags (0, 0)
#endif
/* The various cores that implement AArch64. */
diff --git a/gcc/config/aarch64/aarch64.h b/gcc/config/aarch64/aarch64.h
index dd3437214e1597f03ac947a09c124ea0b04e27e8..12e5b244f28ab04cf1ecc72d2255bea179f97678 100644
--- a/gcc/config/aarch64/aarch64.h
+++ b/gcc/config/aarch64/aarch64.h
@@ -23,17 +23,21 @@
#define GCC_AARCH64_H
#define aarch64_get_asm_isa_flags(opts) \
- (aarch64_feature_flags ((opts)->x_aarch64_asm_isa_flags_0))
+ (aarch64_feature_flags ((opts)->x_aarch64_asm_isa_flags_0, \
+ (opts)->x_aarch64_asm_isa_flags_1))
#define aarch64_get_isa_flags(opts) \
- (aarch64_feature_flags ((opts)->x_aarch64_isa_flags_0))
+ (aarch64_feature_flags ((opts)->x_aarch64_isa_flags_0, \
+ (opts)->x_aarch64_isa_flags_1))
/* Make these flags read-only so that all uses go via
aarch64_set_asm_isa_flags. */
#ifdef GENERATOR_FILE
#undef aarch64_asm_isa_flags
-#define aarch64_asm_isa_flags (aarch64_feature_flags (aarch64_asm_isa_flags_0))
+#define aarch64_asm_isa_flags (aarch64_feature_flags (aarch64_asm_isa_flags_0,\
+ aarch64_asm_isa_flags_1))
#undef aarch64_isa_flags
-#define aarch64_isa_flags (aarch64_feature_flags (aarch64_isa_flags_0))
+#define aarch64_isa_flags (aarch64_feature_flags (aarch64_isa_flags_0, \
+ aarch64_isa_flags_1))
#else
#undef aarch64_asm_isa_flags
#define aarch64_asm_isa_flags (aarch64_get_asm_isa_flags (&global_options))
@@ -209,14 +213,14 @@ constexpr auto AARCH64_ISA_MODE_SM_STATE ATTRIBUTE_UNUSED
/* The mask of all ISA modes. */
constexpr auto AARCH64_FL_ISA_MODES
- = (aarch64_feature_flags (1) << AARCH64_NUM_ISA_MODES) - 1;
+ = aarch64_feature_flags ((1 << AARCH64_NUM_ISA_MODES) - 1, 0);
/* The default ISA mode, for functions with no attributes that specify
something to the contrary. */
constexpr auto AARCH64_DEFAULT_ISA_MODE ATTRIBUTE_UNUSED
= AARCH64_ISA_MODE_SM_OFF;
constexpr auto AARCH64_FL_DEFAULT_ISA_MODE ATTRIBUTE_UNUSED
- = aarch64_feature_flags (AARCH64_DEFAULT_ISA_MODE);
+ = aarch64_feature_flags (AARCH64_DEFAULT_ISA_MODE, 0);
#endif
@@ -229,7 +233,7 @@ constexpr auto AARCH64_FL_DEFAULT_ISA_MODE ATTRIBUTE_UNUSED
#define AARCH64_ISA_SM_OFF (aarch64_isa_flags & AARCH64_FL_SM_OFF)
#define AARCH64_ISA_SM_ON (aarch64_isa_flags & AARCH64_FL_SM_ON)
#define AARCH64_ISA_ZA_ON (aarch64_isa_flags & AARCH64_FL_ZA_ON)
-#define AARCH64_ISA_MODE (aarch64_isa_mode) (aarch64_isa_flags & AARCH64_FL_ISA_MODES)
+#define AARCH64_ISA_MODE (aarch64_isa_flags.get_isa_mode())
#define AARCH64_ISA_V8A (aarch64_isa_flags & AARCH64_FL_V8A)
#define AARCH64_ISA_V8_1A (aarch64_isa_flags & AARCH64_FL_V8_1A)
#define AARCH64_ISA_CRC (aarch64_isa_flags & AARCH64_FL_CRC)
diff --git a/gcc/config/aarch64/aarch64.cc b/gcc/config/aarch64/aarch64.cc
index 052cf297e7672abf015a085ab357836cb3b235e4..f9efb462d75b9e536b89ef6d48bc5852a480cb8c 100644
--- a/gcc/config/aarch64/aarch64.cc
+++ b/gcc/config/aarch64/aarch64.cc
@@ -19083,7 +19083,7 @@ aarch64_set_current_function (tree fndecl)
aarch64_pragma_target_parse. */
if (old_tree == new_tree
&& (!fndecl || aarch64_previous_fndecl)
- && (aarch64_isa_mode) (isa_flags & AARCH64_FL_ISA_MODES) == new_isa_mode)
+ && isa_flags.get_isa_mode() == new_isa_mode)
{
gcc_assert (AARCH64_ISA_MODE == new_isa_mode);
return;
@@ -19098,11 +19098,10 @@ aarch64_set_current_function (tree fndecl)
/* The ISA mode can vary based on function type attributes and
function declaration attributes. Make sure that the target
options correctly reflect these attributes. */
- if ((aarch64_isa_mode) (isa_flags & AARCH64_FL_ISA_MODES) != new_isa_mode)
+ if (isa_flags.get_isa_mode() != new_isa_mode)
{
- auto base_flags = (aarch64_asm_isa_flags & ~AARCH64_FL_ISA_MODES);
- aarch64_set_asm_isa_flags (base_flags
- | (aarch64_feature_flags) new_isa_mode);
+ auto new_flags = aarch64_asm_isa_flags.with_isa_mode (new_isa_mode);
+ aarch64_set_asm_isa_flags (new_flags);
aarch64_override_options_internal (&global_options);
new_tree = build_target_option_node (&global_options,
diff --git a/gcc/config/aarch64/aarch64.opt b/gcc/config/aarch64/aarch64.opt
index 45aab49de27bdfa0fb3f67ec06c7dcf0ac242fb3..2f90f10352af75f70112d07894ab200f48b143f4 100644
--- a/gcc/config/aarch64/aarch64.opt
+++ b/gcc/config/aarch64/aarch64.opt
@@ -33,9 +33,15 @@ enum aarch64_arch selected_arch = aarch64_no_arch
TargetVariable
uint64_t aarch64_asm_isa_flags_0 = 0
+TargetVariable
+uint64_t aarch64_asm_isa_flags_1 = 0
+
TargetVariable
uint64_t aarch64_isa_flags_0 = 0
+TargetVariable
+uint64_t aarch64_isa_flags_1 = 0
+
TargetVariable
unsigned aarch_enable_bti = 2
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 00/12] aarch64: Extend aarch64_feature_flags to 128 bits
2024-05-14 14:54 [PATCH 00/12] aarch64: Extend aarch64_feature_flags to 128 bits Andrew Carlotti
` (11 preceding siblings ...)
2024-05-14 15:00 ` [PATCH 12/12] aarch64: Extend aarch64_feature_flags to 128 bits Andrew Carlotti
@ 2024-05-17 15:45 ` Richard Sandiford
2024-05-20 12:09 ` Andrew Carlotti
12 siblings, 1 reply; 16+ messages in thread
From: Richard Sandiford @ 2024-05-17 15:45 UTC (permalink / raw)
To: Andrew Carlotti; +Cc: gcc-patches
Andrew Carlotti <andrew.carlotti@arm.com> writes:
> The end goal of the series is to change the definition of aarch64_feature_flags
> from a uint64_t typedef to a class with 128 bits of storage. This class uses
> operator overloading to mimic the existing integer interface as much as
> possible, but with added restrictions to facilate type checking and
> extensibility.
>
> Patches 01-10 are preliminary enablement work, and have passed regression
> testing. Are these ok for master?
>
> Patch 11 is an RFC, and the only patch that touches the middle end. I am
> seeking clarity on which part(s) of the compiler should be expected to handle
> or prevent non-bool types in instruction pattern conditions. The actual patch
> does not compile by itself (though it does in combination with 12/12), but that
> is not important to the questions I'm asking.
>
> Patch 12 is then a small patch that actually replaces the uint64_t typedef with
> a class. I think this patch is fine in it's current form, but it depends on a
> resolution to the issues in patch 11/12 first.
Thanks for doing this.
Rather than disallowing flags == 0, etc., I think we should allow
aarch64_feature_flags to be constructed from a single uint64_t.
It's a lossless conversion. The important thing is that we don't
allow conversions the other way (and the patch doesn't allow them).
Also, I think we should make the new class in 12/12 be a templated
<size_t N> type that provides an N-bit bitmask. It should arguably
also be target-independent code. aarch64_feature_flags would then be
an alias with the appropriate number of bits.
For the RFC in 11/12, how about, as another prepatch before 12/12,
removing all the mechanical:
#define AARCH64_ISA_LS64 (aarch64_isa_flags & AARCH64_FL_LS64)
style macros and replacing uses with something like:
AARCH64_HAVE_ISA (LS64)
Uses outside aarch64.h should arguably be changed to TARGET_* instead,
since the convention seems to be that TARGET_* checks the underlying
ISA flag and also any other relevant conditions (where applicable).
Thanks,
Richard
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 00/12] aarch64: Extend aarch64_feature_flags to 128 bits
2024-05-17 15:45 ` [PATCH 00/12] " Richard Sandiford
@ 2024-05-20 12:09 ` Andrew Carlotti
2024-05-20 15:53 ` Richard Sandiford
0 siblings, 1 reply; 16+ messages in thread
From: Andrew Carlotti @ 2024-05-20 12:09 UTC (permalink / raw)
To: gcc-patches, richard.sandiford
On Fri, May 17, 2024 at 04:45:05PM +0100, Richard Sandiford wrote:
> Andrew Carlotti <andrew.carlotti@arm.com> writes:
> > The end goal of the series is to change the definition of aarch64_feature_flags
> > from a uint64_t typedef to a class with 128 bits of storage. This class uses
> > operator overloading to mimic the existing integer interface as much as
> > possible, but with added restrictions to facilate type checking and
> > extensibility.
> >
> > Patches 01-10 are preliminary enablement work, and have passed regression
> > testing. Are these ok for master?
> >
> > Patch 11 is an RFC, and the only patch that touches the middle end. I am
> > seeking clarity on which part(s) of the compiler should be expected to handle
> > or prevent non-bool types in instruction pattern conditions. The actual patch
> > does not compile by itself (though it does in combination with 12/12), but that
> > is not important to the questions I'm asking.
> >
> > Patch 12 is then a small patch that actually replaces the uint64_t typedef with
> > a class. I think this patch is fine in it's current form, but it depends on a
> > resolution to the issues in patch 11/12 first.
>
> Thanks for doing this.
>
> Rather than disallowing flags == 0, etc., I think we should allow
> aarch64_feature_flags to be constructed from a single uint64_t.
> It's a lossless conversion. The important thing is that we don't
> allow conversions the other way (and the patch doesn't allow them).
I agree that allowing conversion from a single int should be safe (albeit it
was probably helpful to disallow it during the development of this series).
It does feel a little bit strange to have a separate mechanism for
setting the first 64 bits (and zeroing the rest).
Do you consider the existing code in some places to be clearer than the new
versions in this patch series? If so, it would be helpful to know which
patches (or parts of patches) I should drop.
> Also, I think we should make the new class in 12/12 be a templated
> <size_t N> type that provides an N-bit bitmask. It should arguably
> also be target-independent code. aarch64_feature_flags would then be
> an alias with the appropriate number of bits.
I think the difficult part is to do this for generic N while still satisfying
C++11 constexpr function requirements (we can't use a loop, for example).
However, while writing this response, I've realised that I can do this using
recursion, with an N-bit bitmask being implemented as a class containing an
M-bit integer and (recursively) and (N-M)-bit bitmask.
> For the RFC in 11/12, how about, as another prepatch before 12/12,
> removing all the mechanical:
>
> #define AARCH64_ISA_LS64 (aarch64_isa_flags & AARCH64_FL_LS64)
>
> style macros and replacing uses with something like:
>
> AARCH64_HAVE_ISA (LS64)
This sounds like a good approach, and is roughly what I was already planning to
do (although I hadn't worked out the details yet). I think that can entirely
replace 11/12 in the context of this series, but the questions about
instruction pattern condition type checking still ought to be addressed
separately.
> Uses outside aarch64.h should arguably be changed to TARGET_* instead,
> since the convention seems to be that TARGET_* checks the underlying
> ISA flag and also any other relevant conditions (where applicable).
>
> Thanks,
> Richard
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 00/12] aarch64: Extend aarch64_feature_flags to 128 bits
2024-05-20 12:09 ` Andrew Carlotti
@ 2024-05-20 15:53 ` Richard Sandiford
0 siblings, 0 replies; 16+ messages in thread
From: Richard Sandiford @ 2024-05-20 15:53 UTC (permalink / raw)
To: Andrew Carlotti; +Cc: gcc-patches
Andrew Carlotti <andrew.carlotti@arm.com> writes:
> On Fri, May 17, 2024 at 04:45:05PM +0100, Richard Sandiford wrote:
>> Andrew Carlotti <andrew.carlotti@arm.com> writes:
>> > The end goal of the series is to change the definition of aarch64_feature_flags
>> > from a uint64_t typedef to a class with 128 bits of storage. This class uses
>> > operator overloading to mimic the existing integer interface as much as
>> > possible, but with added restrictions to facilate type checking and
>> > extensibility.
>> >
>> > Patches 01-10 are preliminary enablement work, and have passed regression
>> > testing. Are these ok for master?
>> >
>> > Patch 11 is an RFC, and the only patch that touches the middle end. I am
>> > seeking clarity on which part(s) of the compiler should be expected to handle
>> > or prevent non-bool types in instruction pattern conditions. The actual patch
>> > does not compile by itself (though it does in combination with 12/12), but that
>> > is not important to the questions I'm asking.
>> >
>> > Patch 12 is then a small patch that actually replaces the uint64_t typedef with
>> > a class. I think this patch is fine in it's current form, but it depends on a
>> > resolution to the issues in patch 11/12 first.
>>
>> Thanks for doing this.
>>
>> Rather than disallowing flags == 0, etc., I think we should allow
>> aarch64_feature_flags to be constructed from a single uint64_t.
>> It's a lossless conversion. The important thing is that we don't
>> allow conversions the other way (and the patch doesn't allow them).
>
> I agree that allowing conversion from a single int should be safe (albeit it
> was probably helpful to disallow it during the development of this series).
> It does feel a little bit strange to have a separate mechanism for
> setting the first 64 bits (and zeroing the rest).
With a templated class, I think it makes sense. The constructor would
take a variable number of arguments and any unspecified elements would
implicitly be zero. In that sense, a single uint64_t isn't a special
case. It's just an instance of a generic rule.
> Do you consider the existing code in some places to be clearer than the new
> versions in this patch series? If so, it would be helpful to know which
> patches (or parts of patches) I should drop.
Probably patches 3, 4, and (for unrelated reasons) 9. (9 feels like
a microoptimisation, given that the underlying issue has been fixed.)
>> Also, I think we should make the new class in 12/12 be a templated
>> <size_t N> type that provides an N-bit bitmask. It should arguably
>> also be target-independent code. aarch64_feature_flags would then be
>> an alias with the appropriate number of bits.
>
> I think the difficult part is to do this for generic N while still satisfying
> C++11 constexpr function requirements (we can't use a loop, for example).
> However, while writing this response, I've realised that I can do this using
> recursion, with an N-bit bitmask being implemented as a class containing an
> M-bit integer and (recursively) and (N-M)-bit bitmask.
I think it'd be better to keep a flat object, not least for debugging.
Things like operator| could be handled using code like:
----------------------------------------------------------------
template<int N>
struct operators
{
template<typename Result, typename Operator, typename Arg, typename ...Rest>
static constexpr Result binary(Operator op, const Arg &x, const Arg &y,
Rest ...rest)
{
return operators<N - 1>::template binary<Result, Operator, Arg>
(op, x, y, op (x[N - 1], y[N - 1]), rest...);
}
};
template<>
struct operators<0>
{
template<typename Result, typename Operator, typename Arg, typename ...Rest>
static constexpr Result binary(Operator op, const Arg &x, const Arg &y,
Rest ...rest)
{
return Result { rest... };
}
};
using T = std::array<int, 2>;
template<typename T>
constexpr T f(T x, T y) { return x | y; }
constexpr T x = { 1, 2 };
constexpr T y = { 0x100, 0x400 };
constexpr T z = operators<2>::binary<T> (f<int>, x, y);
----------------------------------------------------------------
(Unfortunately, constexpr lambdas are also not supported in C++11.)
>> For the RFC in 11/12, how about, as another prepatch before 12/12,
>> removing all the mechanical:
>>
>> #define AARCH64_ISA_LS64 (aarch64_isa_flags & AARCH64_FL_LS64)
>>
>> style macros and replacing uses with something like:
>>
>> AARCH64_HAVE_ISA (LS64)
>
> This sounds like a good approach, and is roughly what I was already planning to
> do (although I hadn't worked out the details yet). I think that can entirely
> replace 11/12 in the context of this series, but the questions about
> instruction pattern condition type checking still ought to be addressed
> separately.
Yeah, stronger typing would be good. I think in practice the generators
should add the "bool (...)" wrapper.
Thanks,
Richard
^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2024-05-20 15:53 UTC | newest]
Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-05-14 14:54 [PATCH 00/12] aarch64: Extend aarch64_feature_flags to 128 bits Andrew Carlotti
2024-05-14 14:55 ` [PATCH 01/12] aarch64: Remove unused global aarch64_tune_flags Andrew Carlotti
2024-05-14 14:55 ` [PATCH 02/12] aarch64: Move AARCH64_NUM_ISA_MODES definition Andrew Carlotti
2024-05-14 14:56 ` [PATCH 03/12] aarch64: Don't use 0 for aarch64_feature_flags Andrew Carlotti
2024-05-14 14:56 ` [PATCH 04/12] aarch64: Don't compare aarch64_feature_flags to 0 Andrew Carlotti
2024-05-14 14:56 ` [PATCH 05/12] aarch64: Eliminate a temporary variable Andrew Carlotti
2024-05-14 14:57 ` [PATCH 06/12] aarch64: Introduce aarch64_isa_mode type Andrew Carlotti
2024-05-14 14:57 ` [PATCH 07/12] aarch64: Define aarch64_get_{asm_|}isa_flags Andrew Carlotti
2024-05-14 14:58 ` [PATCH 08/12] aarch64: Decouple feature flag option storage type Andrew Carlotti
2024-05-14 14:58 ` [PATCH 09/12] aarch64: Assign flags to local constexpr variable Andrew Carlotti
2024-05-14 14:59 ` [PATCH 10/12] aarch64: Add aarch64_feature_flags_from_index macro Andrew Carlotti
2024-05-14 14:59 ` [RFC 11/12] Add explicit bool casts to .md condition users Andrew Carlotti
2024-05-14 15:00 ` [PATCH 12/12] aarch64: Extend aarch64_feature_flags to 128 bits Andrew Carlotti
2024-05-17 15:45 ` [PATCH 00/12] " Richard Sandiford
2024-05-20 12:09 ` Andrew Carlotti
2024-05-20 15:53 ` Richard Sandiford
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).