Re: [PATCH] genopinit: Allow more than 256 modes.
Robin Dapp writes:
> Ok so the consensus seems to rather stay with 32 bits and only
> change the shift to 10/20?
Yeah. The check would then be:
if (NUM_OPTABS > 0xfff || NUM_MACHINE_MODES > 0x3ff)
fatal ("genopinit range assumptions invalid");
> As MACHINE_MODE_BITSIZE is already
> 16 we would need an additional check independent of that.
> Wouldn't that also be a bit confusing?
MACHINE_MODE_BITSIZE isn't really relevant to genopinit.cc.
If we wanted to make it relevant, we ought to express everything
that depends on the mode size in terms of MACHINE_MODE_BITSIZE,
so that the shifts would be << MACHINE_MODE_BITSIZE and
<< (2 * MACHINE_MODE_BITSIZE). The NUM_OPTABS condition would
need to be expressed in terms of MACHINE_MODE_BITSIZE too.
But that seems overkill. The reason for going straight from 8 to 16 for
the bitfields was to keep the accesses fast. But that isn't an issue
for these shifts, since the end result is just ORed into a 32-bit blob.
And 32 bits are still plenty, even with the new RISC-V modes.
Thanks,
Richard
Re: Re: [PATCH] genopinit: Allow more than 256 modes.
Sorry for sending incorrect email.
Forget about this:).
juzhe.zh...@rivai.ai
From: 钟居哲
Date: 2023-07-11 21:55
To: rdapp.gcc; gcc-patches; Jeff Law; richard.sandiford; rguenther
CC: rdapp.gcc
Subject: Re: [PATCH] genopinit: Allow more than 256 modes.
For example:
https://godbolt.org/z/1d6v5WKhY
Clang can vectorize but GCC failed even with -ffast-math.
So I think conversions should be well checked again to make sure every variant
can vectorize.
Thanks.
juzhe.zh...@rivai.ai
From: Robin Dapp
Date: 2023-07-11 21:31
To: Robin Dapp via Gcc-patches; jeffreyalaw; juzhe.zh...@rivai.ai;
richard.sandiford; Richard Biener
CC: rdapp.gcc
Subject: [PATCH] genopinit: Allow more than 256 modes.
Ok so the consensus seems to rather stay with 32 bits and only
change the shift to 10/20? As MACHINE_MODE_BITSIZE is already
16 we would need an additional check independent of that.
Wouldn't that also be a bit confusing?
Attached is a "v2" with unsigned long long changed to
uint64_t and checking for NUM_OPTABS <= 0x.
Regards
Robin
Upcoming changes for RISC-V will have us exceed 256 modes or 8 bits. The
helper functions in gen* rely on the opcode as well as two modes fitting
into an unsigned int (a signed int even if we consider the qsort default
comparison function). This patch changes the type of the index/hash
from unsigned int to uint64_t and allows up to 16 bits for a mode as well
as 32 bits for an optab.
Despite fearing worse, bootstrap, build and test suite runtimes are
actually unchanged (on 64-bit architectures that is).
gcc/ChangeLog:
* genopinit.cc (pattern_cmp): Use if/else for comparison instead
of subtraction.
(main): Change to uint64_t.
* gensupport.cc (find_optab): Ditto.
* gensupport.h (struct optab_pattern): Ditto.
* optabs-query.h (optab_handler): Ditto.
(convert_optab_handler): Ditto.
---
gcc/genopinit.cc | 21 +
gcc/gensupport.cc | 2 +-
gcc/gensupport.h | 2 +-
gcc/optabs-query.h | 4 ++--
4 files changed, 17 insertions(+), 12 deletions(-)
diff --git a/gcc/genopinit.cc b/gcc/genopinit.cc
index 6bd8858a1d9..05316ccb409 100644
--- a/gcc/genopinit.cc
+++ b/gcc/genopinit.cc
@@ -51,7 +51,12 @@ pattern_cmp (const void *va, const void *vb)
{
const optab_pattern *a = (const optab_pattern *)va;
const optab_pattern *b = (const optab_pattern *)vb;
- return a->sort_num - b->sort_num;
+ if (a->sort_num > b->sort_num)
+return 1;
+ else if (a->sort_num < b->sort_num)
+return -1;
+ else
+return 0;
}
static int
@@ -182,7 +187,7 @@ main (int argc, const char **argv)
progname = "genopinit";
- if (NUM_OPTABS > 0x
+ if (NUM_OPTABS > 0x
|| MAX_MACHINE_MODE >= ((1 << MACHINE_MODE_BITSIZE) - 1))
fatal ("genopinit range assumptions inv
Upcoming changes for RISC-V will have us exceed 256 modes or 8 bits. The
helper functions in gen* rely on the opcode as well as two modes fitting
into an unsigned int (a signed int even if we consider the qsort default
comparison function). This patch changes the type of the index/hash
from unsigned int to uint64_t and allows up to 16 bits for a mode as well
as 32 bits for an optab.alid");
@@ -306,7 +311,7 @@ main (int argc, const char **argv)
"extern const struct optab_libcall_d normlib_def[NUM_NORMLIB_OPTABS];\n"
"\n"
"/* Returns the active icode for the given (encoded) optab. */\n"
-"extern enum insn_code raw_optab_handler (unsigned);\n"
+"extern enum insn_code raw_optab_handler (uint64_t);\n"
"extern bool swap_optab_enable (optab, machine_mode, bool);\n"
"\n"
"/* Target-dependent globals. */\n"
@@ -358,14 +363,14 @@ main (int argc, const char **argv)
"#include \"optabs.h\"\n"
"\n"
"struct optab_pat {\n"
-" unsigned scode;\n"
+" uint64_t scode;\n"
" enum insn_code icode;\n"
"};\n\n");
fprintf (s_file,
"static const struct optab_pat pats[NUM_OPTAB_PATTERNS] = {\n");
for (i = 0; patterns.iterate (i, ); ++i)
-fprintf (s_file, " { %#08x, CODE_FOR_%s },\n", p->sort_num, p->name);
+fprintf (s_file, " { %#08llx, CODE_FOR_%s },\n", p->sort_num, p->name);
fprintf (s_file, "};\n\n");
fprintf (s_file, "void\ninit_all_optabs (struct target_optabs
*optabs)\n{\n");
@@ -410,7 +415,7 @@ main (int argc, const char **argv)
the hash entries, which complicates the pat_enable array. */
fprintf (s_file,
"static int\n"
-"lookup_handler (unsigned scode)\n"
+"lookup_handler (uint64_t scode)\n"
"{\n"
" int l = 0, h = ARRAY_SIZE (pats), m;\n"
" while (h > l)\n"
@@ -428,7 +433,7 @@ main (int argc, const char **argv)
fprintf (s_file,
"enum
Re: [PATCH] genopinit: Allow more than 256 modes.
For example:
https://godbolt.org/z/1d6v5WKhY
Clang can vectorize but GCC failed even with -ffast-math.
So I think conversions should be well checked again to make sure every variant
can vectorize.
Thanks.
juzhe.zh...@rivai.ai
From: Robin Dapp
Date: 2023-07-11 21:31
To: Robin Dapp via Gcc-patches; jeffreyalaw; juzhe.zh...@rivai.ai;
richard.sandiford; Richard Biener
CC: rdapp.gcc
Subject: [PATCH] genopinit: Allow more than 256 modes.
Ok so the consensus seems to rather stay with 32 bits and only
change the shift to 10/20? As MACHINE_MODE_BITSIZE is already
16 we would need an additional check independent of that.
Wouldn't that also be a bit confusing?
Attached is a "v2" with unsigned long long changed to
uint64_t and checking for NUM_OPTABS <= 0x.
Regards
Robin
Upcoming changes for RISC-V will have us exceed 256 modes or 8 bits. The
helper functions in gen* rely on the opcode as well as two modes fitting
into an unsigned int (a signed int even if we consider the qsort default
comparison function). This patch changes the type of the index/hash
from unsigned int to uint64_t and allows up to 16 bits for a mode as well
as 32 bits for an optab.
Despite fearing worse, bootstrap, build and test suite runtimes are
actually unchanged (on 64-bit architectures that is).
gcc/ChangeLog:
* genopinit.cc (pattern_cmp): Use if/else for comparison instead
of subtraction.
(main): Change to uint64_t.
* gensupport.cc (find_optab): Ditto.
* gensupport.h (struct optab_pattern): Ditto.
* optabs-query.h (optab_handler): Ditto.
(convert_optab_handler): Ditto.
---
gcc/genopinit.cc | 21 +
gcc/gensupport.cc | 2 +-
gcc/gensupport.h | 2 +-
gcc/optabs-query.h | 4 ++--
4 files changed, 17 insertions(+), 12 deletions(-)
diff --git a/gcc/genopinit.cc b/gcc/genopinit.cc
index 6bd8858a1d9..05316ccb409 100644
--- a/gcc/genopinit.cc
+++ b/gcc/genopinit.cc
@@ -51,7 +51,12 @@ pattern_cmp (const void *va, const void *vb)
{
const optab_pattern *a = (const optab_pattern *)va;
const optab_pattern *b = (const optab_pattern *)vb;
- return a->sort_num - b->sort_num;
+ if (a->sort_num > b->sort_num)
+return 1;
+ else if (a->sort_num < b->sort_num)
+return -1;
+ else
+return 0;
}
static int
@@ -182,7 +187,7 @@ main (int argc, const char **argv)
progname = "genopinit";
- if (NUM_OPTABS > 0x
+ if (NUM_OPTABS > 0x
|| MAX_MACHINE_MODE >= ((1 << MACHINE_MODE_BITSIZE) - 1))
fatal ("genopinit range assumptions inv
Upcoming changes for RISC-V will have us exceed 256 modes or 8 bits. The
helper functions in gen* rely on the opcode as well as two modes fitting
into an unsigned int (a signed int even if we consider the qsort default
comparison function). This patch changes the type of the index/hash
from unsigned int to uint64_t and allows up to 16 bits for a mode as well
as 32 bits for an optab.alid");
@@ -306,7 +311,7 @@ main (int argc, const char **argv)
"extern const struct optab_libcall_d normlib_def[NUM_NORMLIB_OPTABS];\n"
"\n"
"/* Returns the active icode for the given (encoded) optab. */\n"
-"extern enum insn_code raw_optab_handler (unsigned);\n"
+"extern enum insn_code raw_optab_handler (uint64_t);\n"
"extern bool swap_optab_enable (optab, machine_mode, bool);\n"
"\n"
"/* Target-dependent globals. */\n"
@@ -358,14 +363,14 @@ main (int argc, const char **argv)
"#include \"optabs.h\"\n"
"\n"
"struct optab_pat {\n"
-" unsigned scode;\n"
+" uint64_t scode;\n"
" enum insn_code icode;\n"
"};\n\n");
fprintf (s_file,
"static const struct optab_pat pats[NUM_OPTAB_PATTERNS] = {\n");
for (i = 0; patterns.iterate (i, ); ++i)
-fprintf (s_file, " { %#08x, CODE_FOR_%s },\n", p->sort_num, p->name);
+fprintf (s_file, " { %#08llx, CODE_FOR_%s },\n", p->sort_num, p->name);
fprintf (s_file, "};\n\n");
fprintf (s_file, "void\ninit_all_optabs (struct target_optabs
*optabs)\n{\n");
@@ -410,7 +415,7 @@ main (int argc, const char **argv)
the hash entries, which complicates the pat_enable array. */
fprintf (s_file,
"static int\n"
-"lookup_handler (unsigned scode)\n"
+"lookup_handler (uint64_t scode)\n"
"{\n"
" int l = 0, h = ARRAY_SIZE (pats), m;\n"
" while (h > l)\n"
@@ -428,7 +433,7 @@ main (int argc, const char **argv)
fprintf (s_file,
"enum insn_code\n"
-"raw_optab_handler (unsigned scode)\n"
+"raw_optab_handler (uint64_t scode)\n"
"{\n"
" int i = lookup_handler (scode);\n"
" return (i >= 0 && this_fn_optabs->
[PATCH] genopinit: Allow more than 256 modes.
Ok so the consensus seems to rather stay with 32 bits and only
change the shift to 10/20? As MACHINE_MODE_BITSIZE is already
16 we would need an additional check independent of that.
Wouldn't that also be a bit confusing?
Attached is a "v2" with unsigned long long changed to
uint64_t and checking for NUM_OPTABS <= 0x.
Regards
Robin
Upcoming changes for RISC-V will have us exceed 256 modes or 8 bits. The
helper functions in gen* rely on the opcode as well as two modes fitting
into an unsigned int (a signed int even if we consider the qsort default
comparison function). This patch changes the type of the index/hash
from unsigned int to uint64_t and allows up to 16 bits for a mode as well
as 32 bits for an optab.
Despite fearing worse, bootstrap, build and test suite runtimes are
actually unchanged (on 64-bit architectures that is).
gcc/ChangeLog:
* genopinit.cc (pattern_cmp): Use if/else for comparison instead
of subtraction.
(main): Change to uint64_t.
* gensupport.cc (find_optab): Ditto.
* gensupport.h (struct optab_pattern): Ditto.
* optabs-query.h (optab_handler): Ditto.
(convert_optab_handler): Ditto.
---
gcc/genopinit.cc | 21 +
gcc/gensupport.cc | 2 +-
gcc/gensupport.h | 2 +-
gcc/optabs-query.h | 4 ++--
4 files changed, 17 insertions(+), 12 deletions(-)
diff --git a/gcc/genopinit.cc b/gcc/genopinit.cc
index 6bd8858a1d9..05316ccb409 100644
--- a/gcc/genopinit.cc
+++ b/gcc/genopinit.cc
@@ -51,7 +51,12 @@ pattern_cmp (const void *va, const void *vb)
{
const optab_pattern *a = (const optab_pattern *)va;
const optab_pattern *b = (const optab_pattern *)vb;
- return a->sort_num - b->sort_num;
+ if (a->sort_num > b->sort_num)
+return 1;
+ else if (a->sort_num < b->sort_num)
+return -1;
+ else
+return 0;
}
static int
@@ -182,7 +187,7 @@ main (int argc, const char **argv)
progname = "genopinit";
- if (NUM_OPTABS > 0x
+ if (NUM_OPTABS > 0x
|| MAX_MACHINE_MODE >= ((1 << MACHINE_MODE_BITSIZE) - 1))
fatal ("genopinit range assumptions inv
Upcoming changes for RISC-V will have us exceed 256 modes or 8 bits. The
helper functions in gen* rely on the opcode as well as two modes fitting
into an unsigned int (a signed int even if we consider the qsort default
comparison function). This patch changes the type of the index/hash
from unsigned int to uint64_t and allows up to 16 bits for a mode as well
as 32 bits for an optab.alid");
@@ -306,7 +311,7 @@ main (int argc, const char **argv)
"extern const struct optab_libcall_d
normlib_def[NUM_NORMLIB_OPTABS];\n"
"\n"
"/* Returns the active icode for the given (encoded) optab. */\n"
- "extern enum insn_code raw_optab_handler (unsigned);\n"
+ "extern enum insn_code raw_optab_handler (uint64_t);\n"
"extern bool swap_optab_enable (optab, machine_mode, bool);\n"
"\n"
"/* Target-dependent globals. */\n"
@@ -358,14 +363,14 @@ main (int argc, const char **argv)
"#include \"optabs.h\"\n"
"\n"
"struct optab_pat {\n"
- " unsigned scode;\n"
+ " uint64_t scode;\n"
" enum insn_code icode;\n"
"};\n\n");
fprintf (s_file,
"static const struct optab_pat pats[NUM_OPTAB_PATTERNS] = {\n");
for (i = 0; patterns.iterate (i, ); ++i)
-fprintf (s_file, " { %#08x, CODE_FOR_%s },\n", p->sort_num, p->name);
+fprintf (s_file, " { %#08llx, CODE_FOR_%s },\n", p->sort_num, p->name);
fprintf (s_file, "};\n\n");
fprintf (s_file, "void\ninit_all_optabs (struct target_optabs
*optabs)\n{\n");
@@ -410,7 +415,7 @@ main (int argc, const char **argv)
the hash entries, which complicates the pat_enable array. */
fprintf (s_file,
"static int\n"
- "lookup_handler (unsigned scode)\n"
+ "lookup_handler (uint64_t scode)\n"
"{\n"
" int l = 0, h = ARRAY_SIZE (pats), m;\n"
" while (h > l)\n"
@@ -428,7 +433,7 @@ main (int argc, const char **argv)
fprintf (s_file,
"enum insn_code\n"
- "raw_optab_handler (unsigned scode)\n"
+ "raw_optab_handler (uint64_t scode)\n"
"{\n"
" int i = lookup_handler (scode);\n"
" return (i >= 0 && this_fn_optabs->pat_enable[i]\n"
@@ -439,7 +444,7 @@ main (int argc, const char **argv)
"bool\n"
"swap_optab_enable (optab op, machine_mode m, bool set)\n"
"{\n"
- " unsigned scode = (op << 16) | m;\n"
+ " uint64_t scode = ((uint64_t)op << 32) | m;\n"
" int i = lookup_handler (scode);\n"
" if (i >= 0)\n"
"{\n"
diff --git a/gcc/gensupport.cc b/gcc/gensupport.cc
index e39e6dacce2..68df90fce58 100644
--- a/gcc/gensupport.cc
+++ b/gcc/gensupport.cc
@@ -3806,7 +3806,7 @@ find_optab (optab_pattern *p, const char *name)
Re: [PATCH] genopinit: Allow more than 256 modes.
On Tue, 11 Jul 2023, ??? wrote:
> Thanks for fixing it.
> CC Richards to see whether it is appropriate.
I agree with Richard S., but generally please avoid
'long long' and use stdint types when you need specific
precision.
Richard.
>
>
> juzhe.zh...@rivai.ai
>
> From: Robin Dapp
> Date: 2023-07-11 19:51
> To: gcc-patches
> CC: rdapp.gcc; jeffreyalaw; juzhe.zh...@rivai.ai
> Subject: [PATCH] genopinit: Allow more than 256 modes.
> Hi,
>
> upcoming changes for RISC-V will have us exceed 256 modes or 8 bits. The
> helper functions in gen* rely on the opcode as well as two modes fitting
> into an unsigned int (a signed int even if we consider the qsort default
> comparison function). This patch changes the type of the index/hash
> from unsigned int to unsigned long long and allows up to 16 bits for a
> mode as well as 32 bits for an optab.
>
> Despite fearing worse, bootstrap, build and test suite run times on
> x86, aarch64, rv64 and power10 are actually unchanged (I didn't check
> 32-bit architectures but would expect similar results).
>
> Regards
> Robin
>
> gcc/ChangeLog:
>
> * genopinit.cc (pattern_cmp): Use if/else for comparison instead
> of subtraction.
> (main): Change to unsigned long long.
> * gensupport.cc (find_optab): Ditto.
> * gensupport.h (struct optab_pattern): Ditto.
> * optabs-query.h (optab_handler): Ditto.
> (convert_optab_handler): Ditto.
> ---
> gcc/genopinit.cc | 19 ---
> gcc/gensupport.cc | 3 ++-
> gcc/gensupport.h | 2 +-
> gcc/optabs-query.h | 5 +++--
> 4 files changed, 18 insertions(+), 11 deletions(-)
>
> diff --git a/gcc/genopinit.cc b/gcc/genopinit.cc
> index 6bd8858a1d9..58c1bf7cba8 100644
> --- a/gcc/genopinit.cc
> +++ b/gcc/genopinit.cc
> @@ -51,7 +51,12 @@ pattern_cmp (const void *va, const void *vb)
> {
>const optab_pattern *a = (const optab_pattern *)va;
>const optab_pattern *b = (const optab_pattern *)vb;
> - return a->sort_num - b->sort_num;
> + if (a->sort_num > b->sort_num)
> +return 1;
> + else if (a->sort_num < b->sort_num)
> +return -1;
> + else
> +return 0;
> }
> static int
> @@ -306,7 +311,7 @@ main (int argc, const char **argv)
>"extern const struct optab_libcall_d normlib_def[NUM_NORMLIB_OPTABS];\n"
>"\n"
>"/* Returns the active icode for the given (encoded) optab. */\n"
> -"extern enum insn_code raw_optab_handler (unsigned);\n"
> +"extern enum insn_code raw_optab_handler (unsigned long long);\n"
>"extern bool swap_optab_enable (optab, machine_mode, bool);\n"
>"\n"
>"/* Target-dependent globals. */\n"
> @@ -358,14 +363,14 @@ main (int argc, const char **argv)
>"#include \"optabs.h\"\n"
>"\n"
>"struct optab_pat {\n"
> -" unsigned scode;\n"
> +" unsigned long long scode;\n"
>" enum insn_code icode;\n"
>"};\n\n");
>fprintf (s_file,
>"static const struct optab_pat pats[NUM_OPTAB_PATTERNS] = {\n");
>for (i = 0; patterns.iterate (i, ); ++i)
> -fprintf (s_file, " { %#08x, CODE_FOR_%s },\n", p->sort_num, p->name);
> +fprintf (s_file, " { %#08llx, CODE_FOR_%s },\n", p->sort_num, p->name);
>fprintf (s_file, "};\n\n");
>fprintf (s_file, "void\ninit_all_optabs (struct target_optabs
> *optabs)\n{\n");
> @@ -410,7 +415,7 @@ main (int argc, const char **argv)
> the hash entries, which complicates the pat_enable array. */
>fprintf (s_file,
>"static int\n"
> -"lookup_handler (unsigned scode)\n"
> +"lookup_handler (unsigned long long scode)\n"
>"{\n"
>" int l = 0, h = ARRAY_SIZE (pats), m;\n"
>" while (h > l)\n"
> @@ -428,7 +433,7 @@ main (int argc, const char **argv)
>fprintf (s_file,
>"enum insn_code\n"
> -"raw_optab_handler (unsigned scode)\n"
> +"raw_optab_handler (unsigned long long scode)\n"
>"{\n"
>" int i = lookup_handler (scode);\n"
>" return (i >= 0 && this_fn_optabs->pat_enable[i]\n"
> @@ -439,7 +444,7 @@ main (int argc, const char **argv)
>"bool\n"
>"swap_optab_enable (optab op, machine_mode m, bool set)\n"
>"{\n"
> -" unsigned scode = (op << 16) | m;\n"
> +" unsigned long long scode = ((unsigned long long)op << 32) | m;\n"
>" int i = loo
Re: [PATCH] genopinit: Allow more than 256 modes.
> if (NUM_OPTABS > 0x
> || MAX_MACHINE_MODE >= ((1 << MACHINE_MODE_BITSIZE) - 1))
> fatal ("genopinit range assumptions invalid");
>
> so it would be a case of changing those instead.
Thanks, right at the beginning of the file and I didn't see it ;)
MACHINE_MODE_BITSIZE is already 16, Pan changed that for one of
the previous patches. Should we bump the NUM_OPTABS to
0x now, i.e. in this patch or only when the need arises?
Regards
Robin
Re: [PATCH] genopinit: Allow more than 256 modes.
Richard Sandiford writes:
> Robin Dapp via Gcc-patches writes:
>> Hi,
>>
>> upcoming changes for RISC-V will have us exceed 256 modes or 8 bits. The
>> helper functions in gen* rely on the opcode as well as two modes fitting
>> into an unsigned int (a signed int even if we consider the qsort default
>> comparison function). This patch changes the type of the index/hash
>> from unsigned int to unsigned long long and allows up to 16 bits for a
>> mode as well as 32 bits for an optab.
>>
>> Despite fearing worse, bootstrap, build and test suite run times on
>> x86, aarch64, rv64 and power10 are actually unchanged (I didn't check
>> 32-bit architectures but would expect similar results).
>
> I think for now we should just bump the mode shift to 10 and assert
> (statically) that MAX_MACHINE_MODE < 1024.
Sorry, just remembered that we already have:
if (NUM_OPTABS > 0x
|| MAX_MACHINE_MODE >= ((1 << MACHINE_MODE_BITSIZE) - 1))
fatal ("genopinit range assumptions invalid");
so it would be a case of changing those instead.
Thanks,
Richard
Re: [PATCH] genopinit: Allow more than 256 modes.
Robin Dapp via Gcc-patches writes:
> Hi,
>
> upcoming changes for RISC-V will have us exceed 256 modes or 8 bits. The
> helper functions in gen* rely on the opcode as well as two modes fitting
> into an unsigned int (a signed int even if we consider the qsort default
> comparison function). This patch changes the type of the index/hash
> from unsigned int to unsigned long long and allows up to 16 bits for a
> mode as well as 32 bits for an optab.
>
> Despite fearing worse, bootstrap, build and test suite run times on
> x86, aarch64, rv64 and power10 are actually unchanged (I didn't check
> 32-bit architectures but would expect similar results).
I think for now we should just bump the mode shift to 10 and assert
(statically) that MAX_MACHINE_MODE < 1024.
Thanks,
Richard
> Regards
> Robin
>
> gcc/ChangeLog:
>
> * genopinit.cc (pattern_cmp): Use if/else for comparison instead
> of subtraction.
> (main): Change to unsigned long long.
> * gensupport.cc (find_optab): Ditto.
> * gensupport.h (struct optab_pattern): Ditto.
> * optabs-query.h (optab_handler): Ditto.
> (convert_optab_handler): Ditto.
> ---
> gcc/genopinit.cc | 19 ---
> gcc/gensupport.cc | 3 ++-
> gcc/gensupport.h | 2 +-
> gcc/optabs-query.h | 5 +++--
> 4 files changed, 18 insertions(+), 11 deletions(-)
>
> diff --git a/gcc/genopinit.cc b/gcc/genopinit.cc
> index 6bd8858a1d9..58c1bf7cba8 100644
> --- a/gcc/genopinit.cc
> +++ b/gcc/genopinit.cc
> @@ -51,7 +51,12 @@ pattern_cmp (const void *va, const void *vb)
> {
>const optab_pattern *a = (const optab_pattern *)va;
>const optab_pattern *b = (const optab_pattern *)vb;
> - return a->sort_num - b->sort_num;
> + if (a->sort_num > b->sort_num)
> +return 1;
> + else if (a->sort_num < b->sort_num)
> +return -1;
> + else
> +return 0;
> }
>
> static int
> @@ -306,7 +311,7 @@ main (int argc, const char **argv)
> "extern const struct optab_libcall_d
> normlib_def[NUM_NORMLIB_OPTABS];\n"
> "\n"
> "/* Returns the active icode for the given (encoded) optab. */\n"
> -"extern enum insn_code raw_optab_handler (unsigned);\n"
> +"extern enum insn_code raw_optab_handler (unsigned long long);\n"
> "extern bool swap_optab_enable (optab, machine_mode, bool);\n"
> "\n"
> "/* Target-dependent globals. */\n"
> @@ -358,14 +363,14 @@ main (int argc, const char **argv)
> "#include \"optabs.h\"\n"
> "\n"
> "struct optab_pat {\n"
> -" unsigned scode;\n"
> +" unsigned long long scode;\n"
> " enum insn_code icode;\n"
> "};\n\n");
>
>fprintf (s_file,
> "static const struct optab_pat pats[NUM_OPTAB_PATTERNS] = {\n");
>for (i = 0; patterns.iterate (i, ); ++i)
> -fprintf (s_file, " { %#08x, CODE_FOR_%s },\n", p->sort_num, p->name);
> +fprintf (s_file, " { %#08llx, CODE_FOR_%s },\n", p->sort_num, p->name);
>fprintf (s_file, "};\n\n");
>
>fprintf (s_file, "void\ninit_all_optabs (struct target_optabs
> *optabs)\n{\n");
> @@ -410,7 +415,7 @@ main (int argc, const char **argv)
> the hash entries, which complicates the pat_enable array. */
>fprintf (s_file,
> "static int\n"
> -"lookup_handler (unsigned scode)\n"
> +"lookup_handler (unsigned long long scode)\n"
> "{\n"
> " int l = 0, h = ARRAY_SIZE (pats), m;\n"
> " while (h > l)\n"
> @@ -428,7 +433,7 @@ main (int argc, const char **argv)
>
>fprintf (s_file,
> "enum insn_code\n"
> -"raw_optab_handler (unsigned scode)\n"
> +"raw_optab_handler (unsigned long long scode)\n"
> "{\n"
> " int i = lookup_handler (scode);\n"
> " return (i >= 0 && this_fn_optabs->pat_enable[i]\n"
> @@ -439,7 +444,7 @@ main (int argc, const char **argv)
> "bool\n"
> "swap_optab_enable (optab op, machine_mode m, bool set)\n"
> "{\n"
> -" unsigned scode = (op << 16) | m;\n"
> +" unsigned long long scode = ((unsigned long long)op << 32) | m;\n"
> " int i = lookup_handler (scode);\n"
> " if (i >= 0)\n"
> "{\n"
> diff --git a/gcc/gensupport.cc b/gcc/gensupport.cc
> index e39e6dacce2..3fe7428372d 100644
> --- a/gcc/gensupport.cc
> +++ b/gcc/gensupport.cc
> @@ -3806,7 +3806,8 @@ find_optab (optab_pattern *p, const char *name)
> {
> p->name = name;
> p->op = optabs[pindex].op;
> - p->sort_num = (p->op << 16) | (p->m2 << 8) | p->m1;
> + p->sort_num
> + = ((unsigned long long) p->op << 32) | (p->m2 << 16) | p->m1;
> return true;
> }
> }
> diff --git a/gcc/gensupport.h b/gcc/gensupport.h
> index 7925e22ed41..9f70e2310e2 100644
> --- a/gcc/gensupport.h
> +++ b/gcc/gensupport.h
> @@ -123,7 +123,7 @@ struct optab_pattern
>
>/* An index that provides a lexicographical sort of (OP, M2, M1).
>
Re: [PATCH] genopinit: Allow more than 256 modes.
Thanks for fixing it.
CC Richards to see whether it is appropriate.
juzhe.zh...@rivai.ai
From: Robin Dapp
Date: 2023-07-11 19:51
To: gcc-patches
CC: rdapp.gcc; jeffreyalaw; juzhe.zh...@rivai.ai
Subject: [PATCH] genopinit: Allow more than 256 modes.
Hi,
upcoming changes for RISC-V will have us exceed 256 modes or 8 bits. The
helper functions in gen* rely on the opcode as well as two modes fitting
into an unsigned int (a signed int even if we consider the qsort default
comparison function). This patch changes the type of the index/hash
from unsigned int to unsigned long long and allows up to 16 bits for a
mode as well as 32 bits for an optab.
Despite fearing worse, bootstrap, build and test suite run times on
x86, aarch64, rv64 and power10 are actually unchanged (I didn't check
32-bit architectures but would expect similar results).
Regards
Robin
gcc/ChangeLog:
* genopinit.cc (pattern_cmp): Use if/else for comparison instead
of subtraction.
(main): Change to unsigned long long.
* gensupport.cc (find_optab): Ditto.
* gensupport.h (struct optab_pattern): Ditto.
* optabs-query.h (optab_handler): Ditto.
(convert_optab_handler): Ditto.
---
gcc/genopinit.cc | 19 ---
gcc/gensupport.cc | 3 ++-
gcc/gensupport.h | 2 +-
gcc/optabs-query.h | 5 +++--
4 files changed, 18 insertions(+), 11 deletions(-)
diff --git a/gcc/genopinit.cc b/gcc/genopinit.cc
index 6bd8858a1d9..58c1bf7cba8 100644
--- a/gcc/genopinit.cc
+++ b/gcc/genopinit.cc
@@ -51,7 +51,12 @@ pattern_cmp (const void *va, const void *vb)
{
const optab_pattern *a = (const optab_pattern *)va;
const optab_pattern *b = (const optab_pattern *)vb;
- return a->sort_num - b->sort_num;
+ if (a->sort_num > b->sort_num)
+return 1;
+ else if (a->sort_num < b->sort_num)
+return -1;
+ else
+return 0;
}
static int
@@ -306,7 +311,7 @@ main (int argc, const char **argv)
"extern const struct optab_libcall_d normlib_def[NUM_NORMLIB_OPTABS];\n"
"\n"
"/* Returns the active icode for the given (encoded) optab. */\n"
-"extern enum insn_code raw_optab_handler (unsigned);\n"
+"extern enum insn_code raw_optab_handler (unsigned long long);\n"
"extern bool swap_optab_enable (optab, machine_mode, bool);\n"
"\n"
"/* Target-dependent globals. */\n"
@@ -358,14 +363,14 @@ main (int argc, const char **argv)
"#include \"optabs.h\"\n"
"\n"
"struct optab_pat {\n"
-" unsigned scode;\n"
+" unsigned long long scode;\n"
" enum insn_code icode;\n"
"};\n\n");
fprintf (s_file,
"static const struct optab_pat pats[NUM_OPTAB_PATTERNS] = {\n");
for (i = 0; patterns.iterate (i, ); ++i)
-fprintf (s_file, " { %#08x, CODE_FOR_%s },\n", p->sort_num, p->name);
+fprintf (s_file, " { %#08llx, CODE_FOR_%s },\n", p->sort_num, p->name);
fprintf (s_file, "};\n\n");
fprintf (s_file, "void\ninit_all_optabs (struct target_optabs
*optabs)\n{\n");
@@ -410,7 +415,7 @@ main (int argc, const char **argv)
the hash entries, which complicates the pat_enable array. */
fprintf (s_file,
"static int\n"
-"lookup_handler (unsigned scode)\n"
+"lookup_handler (unsigned long long scode)\n"
"{\n"
" int l = 0, h = ARRAY_SIZE (pats), m;\n"
" while (h > l)\n"
@@ -428,7 +433,7 @@ main (int argc, const char **argv)
fprintf (s_file,
"enum insn_code\n"
-"raw_optab_handler (unsigned scode)\n"
+"raw_optab_handler (unsigned long long scode)\n"
"{\n"
" int i = lookup_handler (scode);\n"
" return (i >= 0 && this_fn_optabs->pat_enable[i]\n"
@@ -439,7 +444,7 @@ main (int argc, const char **argv)
"bool\n"
"swap_optab_enable (optab op, machine_mode m, bool set)\n"
"{\n"
-" unsigned scode = (op << 16) | m;\n"
+" unsigned long long scode = ((unsigned long long)op << 32) | m;\n"
" int i = lookup_handler (scode);\n"
" if (i >= 0)\n"
"{\n"
diff --git a/gcc/gensupport.cc b/gcc/gensupport.cc
index e39e6dacce2..3fe7428372d 100644
--- a/gcc/gensupport.cc
+++ b/gcc/gensupport.cc
@@ -3806,7 +3806,8 @@ find_optab (optab_pattern *p, const char *name)
{
p->name = name;
p->op = optabs[pindex].op;
- p->sort_num = (p->op << 16) | (p->m2 << 8) | p->m1;
+ p->sort_num
+ = ((unsigned long long) p->op << 32) | (p->m2 << 16) | p->m1;
return true;
}
}
diff --git a/gcc/gensupport.h b/gcc/gensupport.h
index 7925e22ed41..9f70e2310e2 100644
--- a/gcc/gensupport.h
+++ b/gcc/gensupport.h
@@ -123,7 +123,7 @@ struct
[PATCH] genopinit: Allow more than 256 modes.
Hi,
upcoming changes for RISC-V will have us exceed 256 modes or 8 bits. The
helper functions in gen* rely on the opcode as well as two modes fitting
into an unsigned int (a signed int even if we consider the qsort default
comparison function). This patch changes the type of the index/hash
from unsigned int to unsigned long long and allows up to 16 bits for a
mode as well as 32 bits for an optab.
Despite fearing worse, bootstrap, build and test suite run times on
x86, aarch64, rv64 and power10 are actually unchanged (I didn't check
32-bit architectures but would expect similar results).
Regards
Robin
gcc/ChangeLog:
* genopinit.cc (pattern_cmp): Use if/else for comparison instead
of subtraction.
(main): Change to unsigned long long.
* gensupport.cc (find_optab): Ditto.
* gensupport.h (struct optab_pattern): Ditto.
* optabs-query.h (optab_handler): Ditto.
(convert_optab_handler): Ditto.
---
gcc/genopinit.cc | 19 ---
gcc/gensupport.cc | 3 ++-
gcc/gensupport.h | 2 +-
gcc/optabs-query.h | 5 +++--
4 files changed, 18 insertions(+), 11 deletions(-)
diff --git a/gcc/genopinit.cc b/gcc/genopinit.cc
index 6bd8858a1d9..58c1bf7cba8 100644
--- a/gcc/genopinit.cc
+++ b/gcc/genopinit.cc
@@ -51,7 +51,12 @@ pattern_cmp (const void *va, const void *vb)
{
const optab_pattern *a = (const optab_pattern *)va;
const optab_pattern *b = (const optab_pattern *)vb;
- return a->sort_num - b->sort_num;
+ if (a->sort_num > b->sort_num)
+return 1;
+ else if (a->sort_num < b->sort_num)
+return -1;
+ else
+return 0;
}
static int
@@ -306,7 +311,7 @@ main (int argc, const char **argv)
"extern const struct optab_libcall_d
normlib_def[NUM_NORMLIB_OPTABS];\n"
"\n"
"/* Returns the active icode for the given (encoded) optab. */\n"
- "extern enum insn_code raw_optab_handler (unsigned);\n"
+ "extern enum insn_code raw_optab_handler (unsigned long long);\n"
"extern bool swap_optab_enable (optab, machine_mode, bool);\n"
"\n"
"/* Target-dependent globals. */\n"
@@ -358,14 +363,14 @@ main (int argc, const char **argv)
"#include \"optabs.h\"\n"
"\n"
"struct optab_pat {\n"
- " unsigned scode;\n"
+ " unsigned long long scode;\n"
" enum insn_code icode;\n"
"};\n\n");
fprintf (s_file,
"static const struct optab_pat pats[NUM_OPTAB_PATTERNS] = {\n");
for (i = 0; patterns.iterate (i, ); ++i)
-fprintf (s_file, " { %#08x, CODE_FOR_%s },\n", p->sort_num, p->name);
+fprintf (s_file, " { %#08llx, CODE_FOR_%s },\n", p->sort_num, p->name);
fprintf (s_file, "};\n\n");
fprintf (s_file, "void\ninit_all_optabs (struct target_optabs
*optabs)\n{\n");
@@ -410,7 +415,7 @@ main (int argc, const char **argv)
the hash entries, which complicates the pat_enable array. */
fprintf (s_file,
"static int\n"
- "lookup_handler (unsigned scode)\n"
+ "lookup_handler (unsigned long long scode)\n"
"{\n"
" int l = 0, h = ARRAY_SIZE (pats), m;\n"
" while (h > l)\n"
@@ -428,7 +433,7 @@ main (int argc, const char **argv)
fprintf (s_file,
"enum insn_code\n"
- "raw_optab_handler (unsigned scode)\n"
+ "raw_optab_handler (unsigned long long scode)\n"
"{\n"
" int i = lookup_handler (scode);\n"
" return (i >= 0 && this_fn_optabs->pat_enable[i]\n"
@@ -439,7 +444,7 @@ main (int argc, const char **argv)
"bool\n"
"swap_optab_enable (optab op, machine_mode m, bool set)\n"
"{\n"
- " unsigned scode = (op << 16) | m;\n"
+ " unsigned long long scode = ((unsigned long long)op << 32) | m;\n"
" int i = lookup_handler (scode);\n"
" if (i >= 0)\n"
"{\n"
diff --git a/gcc/gensupport.cc b/gcc/gensupport.cc
index e39e6dacce2..3fe7428372d 100644
--- a/gcc/gensupport.cc
+++ b/gcc/gensupport.cc
@@ -3806,7 +3806,8 @@ find_optab (optab_pattern *p, const char *name)
{
p->name = name;
p->op = optabs[pindex].op;
- p->sort_num = (p->op << 16) | (p->m2 << 8) | p->m1;
+ p->sort_num
+ = ((unsigned long long) p->op << 32) | (p->m2 << 16) | p->m1;
return true;
}
}
diff --git a/gcc/gensupport.h b/gcc/gensupport.h
index 7925e22ed41..9f70e2310e2 100644
--- a/gcc/gensupport.h
+++ b/gcc/gensupport.h
@@ -123,7 +123,7 @@ struct optab_pattern
/* An index that provides a lexicographical sort of (OP, M2, M1).
Used by genopinit.cc. */
- unsigned int sort_num;
+ unsigned long long sort_num;
};
extern rtx add_implicit_parallel (rtvec);
diff --git a/gcc/optabs-query.h b/gcc/optabs-query.h
index 043e9791bc1..5a1d2f75470 100644
--- a/gcc/optabs-query.h
+++ b/gcc/optabs-query.h
@@ -37,7 +37,7 @@
