[PATCH] genopinit: Allow more than 256 modes.

[PATCH] genopinit: Allow more than 256 modes.

[Robin Dapp]

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, &p); ++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 @@ convert_optab_p (optab op)
 inline enum insn_code
 optab_handler (optab op, machine_mode mode)
 {
-  unsigned scode = (op << 16) | mode;
+  unsigned long long scode = ((unsigned long long)op << 32) | mode;
   gcc_assert (op > LAST_CONV_OPTAB);
   return raw_optab_handler (scode);
 }
@@ -50,7 +50,8 @@ inline enum insn_code
 convert_optab_handler (convert_optab op, machine_mode to_mode,
 		       machine_mode from_mode)
 {
-  unsigned scode = (op << 16) | (from_mode << 8) | to_mode;
+  unsigned long long scode
+    = ((unsigned long long) op << 32) | (from_mode << 16) | to_mode;
   gcc_assert (convert_optab_p (op));
   return raw_optab_handler (scode);
 }
-- 
2.41.0

Re: [PATCH] genopinit: Allow more than 256 modes.

[钟居哲]

[-- Attachment #1: Type: text/plain, Size: 5668 bytes --]

Thanks for fixing it.
CC Richards to see whether it is appropriate.



juzhe.zhong@rivai.ai
 
From: Robin Dapp
Date: 2023-07-11 19:51
To: gcc-patches
CC: rdapp.gcc; jeffreyalaw; juzhe.zhong@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, &p); ++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 @@ convert_optab_p (optab op)
inline enum insn_code
optab_handler (optab op, machine_mode mode)
{
-  unsigned scode = (op << 16) | mode;
+  unsigned long long scode = ((unsigned long long)op << 32) | mode;
   gcc_assert (op > LAST_CONV_OPTAB);
   return raw_optab_handler (scode);
}
@@ -50,7 +50,8 @@ inline enum insn_code
convert_optab_handler (convert_optab op, machine_mode to_mode,
       machine_mode from_mode)
{
-  unsigned scode = (op << 16) | (from_mode << 8) | to_mode;
+  unsigned long long scode
+    = ((unsigned long long) op << 32) | (from_mode << 16) | to_mode;
   gcc_assert (convert_optab_p (op));
   return raw_optab_handler (scode);
}
-- 
2.41.0
 

Re: [PATCH] genopinit: Allow more than 256 modes.

[Richard Biener]

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.zhong@rivai.ai
>  
> From: Robin Dapp
> Date: 2023-07-11 19:51
> To: gcc-patches
> CC: rdapp.gcc; jeffreyalaw; juzhe.zhong@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, &p); ++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 @@ convert_optab_p (optab op)
> inline enum insn_code
> optab_handler (optab op, machine_mode mode)
> {
> -  unsigned scode = (op << 16) | mode;
> +  unsigned long long scode = ((unsigned long long)op << 32) | mode;
>    gcc_assert (op > LAST_CONV_OPTAB);
>    return raw_optab_handler (scode);
> }
> @@ -50,7 +50,8 @@ inline enum insn_code
> convert_optab_handler (convert_optab op, machine_mode to_mode,
>        machine_mode from_mode)
> {
> -  unsigned scode = (op << 16) | (from_mode << 8) | to_mode;
> +  unsigned long long scode
> +    = ((unsigned long long) op << 32) | (from_mode << 16) | to_mode;
>    gcc_assert (convert_optab_p (op));
>    return raw_optab_handler (scode);
> }
> 

-- 
Richard Biener <rguenther@suse.de>
SUSE Software Solutions Germany GmbH, Frankenstrasse 146, 90461 Nuernberg,
Germany; GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman;
HRB 36809 (AG Nuernberg)

Re: [PATCH] genopinit: Allow more than 256 modes.

[Richard Sandiford]

Robin Dapp via Gcc-patches <gcc-patches@gcc.gnu.org> 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, &p); ++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 @@ convert_optab_p (optab op)
>  inline enum insn_code
>  optab_handler (optab op, machine_mode mode)
>  {
> -  unsigned scode = (op << 16) | mode;
> +  unsigned long long scode = ((unsigned long long)op << 32) | mode;
>    gcc_assert (op > LAST_CONV_OPTAB);
>    return raw_optab_handler (scode);
>  }
> @@ -50,7 +50,8 @@ inline enum insn_code
>  convert_optab_handler (convert_optab op, machine_mode to_mode,
>  		       machine_mode from_mode)
>  {
> -  unsigned scode = (op << 16) | (from_mode << 8) | to_mode;
> +  unsigned long long scode
> +    = ((unsigned long long) op << 32) | (from_mode << 16) | to_mode;
>    gcc_assert (convert_optab_p (op));
>    return raw_optab_handler (scode);
>  }

Re: [PATCH] genopinit: Allow more than 256 modes.

[Richard Sandiford]

Richard Sandiford <richard.sandiford@arm.com> writes:
> Robin Dapp via Gcc-patches <gcc-patches@gcc.gnu.org> 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 > 0xffff
    || 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]

>   if (NUM_OPTABS > 0xffff
>     || 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
0xffffffff now, i.e. in this patch or only when the need arises?

Regards
 Robin

[PATCH] genopinit: Allow more than 256 modes.

[Robin Dapp]

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 <= 0xffffffff.

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 > 0xffff
+  if (NUM_OPTABS > 0xffffffff
     || 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, &p); ++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)
 	{
 	  p->name = name;
 	  p->op = optabs[pindex].op;
-	  p->sort_num = (p->op << 16) | (p->m2 << 8) | p->m1;
+	  p->sort_num = ((uint64_t) p->op << 32) | (p->m2 << 16) | p->m1;
 	  return true;
 	}
     }
diff --git a/gcc/gensupport.h b/gcc/gensupport.h
index 7925e22ed41..247b375f965 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;
+  uint64_t sort_num;
 };
 
 extern rtx add_implicit_parallel (rtvec);
diff --git a/gcc/optabs-query.h b/gcc/optabs-query.h
index 043e9791bc1..c6b38da5b26 100644
--- a/gcc/optabs-query.h
+++ b/gcc/optabs-query.h
@@ -37,7 +37,7 @@ convert_optab_p (optab op)
 inline enum insn_code
 optab_handler (optab op, machine_mode mode)
 {
-  unsigned scode = (op << 16) | mode;
+  uint64_t scode = ((uint64_t)op << 32) | mode;
   gcc_assert (op > LAST_CONV_OPTAB);
   return raw_optab_handler (scode);
 }
@@ -50,7 +50,7 @@ inline enum insn_code
 convert_optab_handler (convert_optab op, machine_mode to_mode,
 		       machine_mode from_mode)
 {
-  unsigned scode = (op << 16) | (from_mode << 8) | to_mode;
+  uint64_t scode = ((uint64_t) op << 32) | (from_mode << 16) | to_mode;
   gcc_assert (convert_optab_p (op));
   return raw_optab_handler (scode);
 }
-- 
2.41.0

Re: [PATCH] genopinit: Allow more than 256 modes.

[钟居哲]

[-- Attachment #1: Type: text/plain, Size: 6630 bytes --]

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.zhong@rivai.ai
 
From: Robin Dapp
Date: 2023-07-11 21:31
To: Robin Dapp via Gcc-patches; jeffreyalaw; juzhe.zhong@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 <= 0xffffffff.
 
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 > 0xffff
+  if (NUM_OPTABS > 0xffffffff
     || 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, &p); ++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)
{
  p->name = name;
  p->op = optabs[pindex].op;
-   p->sort_num = (p->op << 16) | (p->m2 << 8) | p->m1;
+   p->sort_num = ((uint64_t) p->op << 32) | (p->m2 << 16) | p->m1;
  return true;
}
     }
diff --git a/gcc/gensupport.h b/gcc/gensupport.h
index 7925e22ed41..247b375f965 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;
+  uint64_t sort_num;
};
extern rtx add_implicit_parallel (rtvec);
diff --git a/gcc/optabs-query.h b/gcc/optabs-query.h
index 043e9791bc1..c6b38da5b26 100644
--- a/gcc/optabs-query.h
+++ b/gcc/optabs-query.h
@@ -37,7 +37,7 @@ convert_optab_p (optab op)
inline enum insn_code
optab_handler (optab op, machine_mode mode)
{
-  unsigned scode = (op << 16) | mode;
+  uint64_t scode = ((uint64_t)op << 32) | mode;
   gcc_assert (op > LAST_CONV_OPTAB);
   return raw_optab_handler (scode);
}
@@ -50,7 +50,7 @@ inline enum insn_code
convert_optab_handler (convert_optab op, machine_mode to_mode,
       machine_mode from_mode)
{
-  unsigned scode = (op << 16) | (from_mode << 8) | to_mode;
+  uint64_t scode = ((uint64_t) op << 32) | (from_mode << 16) | to_mode;
   gcc_assert (convert_optab_p (op));
   return raw_optab_handler (scode);
}
-- 
2.41.0
 
 
 

Re: Re: [PATCH] genopinit: Allow more than 256 modes.

[钟居哲]

[-- Attachment #1: Type: text/plain, Size: 6907 bytes --]

Sorry for sending incorrect email.
Forget about this:).....



juzhe.zhong@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.zhong@rivai.ai
 
From: Robin Dapp
Date: 2023-07-11 21:31
To: Robin Dapp via Gcc-patches; jeffreyalaw; juzhe.zhong@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 <= 0xffffffff.
 
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 > 0xffff
+  if (NUM_OPTABS > 0xffffffff
     || 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, &p); ++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)
{
  p->name = name;
  p->op = optabs[pindex].op;
-   p->sort_num = (p->op << 16) | (p->m2 << 8) | p->m1;
+   p->sort_num = ((uint64_t) p->op << 32) | (p->m2 << 16) | p->m1;
  return true;
}
     }
diff --git a/gcc/gensupport.h b/gcc/gensupport.h
index 7925e22ed41..247b375f965 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;
+  uint64_t sort_num;
};
extern rtx add_implicit_parallel (rtvec);
diff --git a/gcc/optabs-query.h b/gcc/optabs-query.h
index 043e9791bc1..c6b38da5b26 100644
--- a/gcc/optabs-query.h
+++ b/gcc/optabs-query.h
@@ -37,7 +37,7 @@ convert_optab_p (optab op)
inline enum insn_code
optab_handler (optab op, machine_mode mode)
{
-  unsigned scode = (op << 16) | mode;
+  uint64_t scode = ((uint64_t)op << 32) | mode;
   gcc_assert (op > LAST_CONV_OPTAB);
   return raw_optab_handler (scode);
}
@@ -50,7 +50,7 @@ inline enum insn_code
convert_optab_handler (convert_optab op, machine_mode to_mode,
       machine_mode from_mode)
{
-  unsigned scode = (op << 16) | (from_mode << 8) | to_mode;
+  uint64_t scode = ((uint64_t) op << 32) | (from_mode << 16) | to_mode;
   gcc_assert (convert_optab_p (op));
   return raw_optab_handler (scode);
}
-- 
2.41.0
 
 
 

Re: [PATCH] genopinit: Allow more than 256 modes.

[Richard Sandiford]

Robin Dapp <rdapp.gcc@gmail.com> 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

[PATCH v2] genopinit: Allow more than 256 modes.

[Robin Dapp]

Attached is v2 that does not switch to uint64_t but stays within
32 bits by shifting the optab by 20 and the mode(s) by 10 bits.

Regards
 Robin

Upcoming changes for RISC-V will have us exceed 255 modes or 8 bits.
This patch increases the limit to 10 bits and adjusts the hashing
function for the gen* and optabs-query lookups accordingly.
Consequently, the number of optabs is limited to 4095.

gcc/ChangeLog:

	* genopinit.cc (main): Adjust maximal number of optabs and
	machine modes.
	* gensupport.cc (find_optab): Shift optab by 20 and mode by
	10 bits.
	* optabs-query.h (optab_handler): Ditto.
	(convert_optab_handler): Ditto.
---
 gcc/genopinit.cc   | 5 ++---
 gcc/gensupport.cc  | 2 +-
 gcc/optabs-query.h | 4 ++--
 3 files changed, 5 insertions(+), 6 deletions(-)

diff --git a/gcc/genopinit.cc b/gcc/genopinit.cc
index 6bd8858a1d9..2a841006884 100644
--- a/gcc/genopinit.cc
+++ b/gcc/genopinit.cc
@@ -182,8 +182,7 @@ main (int argc, const char **argv)
 
   progname = "genopinit";
 
-  if (NUM_OPTABS > 0xffff
-    || MAX_MACHINE_MODE >= ((1 << MACHINE_MODE_BITSIZE) - 1))
+  if (NUM_OPTABS > 0xfff || NUM_MACHINE_MODES > 0x3ff)
     fatal ("genopinit range assumptions invalid");
 
   if (!init_rtx_reader_args_cb (argc, argv, handle_arg))
@@ -439,7 +438,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 scode = (op << 20) | 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..959d1d9c83c 100644
--- a/gcc/gensupport.cc
+++ b/gcc/gensupport.cc
@@ -3806,7 +3806,7 @@ 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 = (p->op << 20) | (p->m2 << 10) | p->m1;
 	  return true;
 	}
     }
diff --git a/gcc/optabs-query.h b/gcc/optabs-query.h
index 043e9791bc1..920eb6a1b67 100644
--- a/gcc/optabs-query.h
+++ b/gcc/optabs-query.h
@@ -37,7 +37,7 @@ convert_optab_p (optab op)
 inline enum insn_code
 optab_handler (optab op, machine_mode mode)
 {
-  unsigned scode = (op << 16) | mode;
+  unsigned scode = (op << 20) | mode;
   gcc_assert (op > LAST_CONV_OPTAB);
   return raw_optab_handler (scode);
 }
@@ -50,7 +50,7 @@ inline enum insn_code
 convert_optab_handler (convert_optab op, machine_mode to_mode,
 		       machine_mode from_mode)
 {
-  unsigned scode = (op << 16) | (from_mode << 8) | to_mode;
+  unsigned scode = (op << 20) | (from_mode << 10) | to_mode;
   gcc_assert (convert_optab_p (op));
   return raw_optab_handler (scode);
 }
-- 
2.41.0

Re: [PATCH v2] genopinit: Allow more than 256 modes.

[Richard Biener]

On Tue, 11 Jul 2023, Robin Dapp wrote:

> Attached is v2 that does not switch to uint64_t but stays within
> 32 bits by shifting the optab by 20 and the mode(s) by 10 bits.

LGTM.

> Regards
>  Robin
> 
> Upcoming changes for RISC-V will have us exceed 255 modes or 8 bits.
> This patch increases the limit to 10 bits and adjusts the hashing
> function for the gen* and optabs-query lookups accordingly.
> Consequently, the number of optabs is limited to 4095.
> 
> gcc/ChangeLog:
> 
> 	* genopinit.cc (main): Adjust maximal number of optabs and
> 	machine modes.
> 	* gensupport.cc (find_optab): Shift optab by 20 and mode by
> 	10 bits.
> 	* optabs-query.h (optab_handler): Ditto.
> 	(convert_optab_handler): Ditto.
> ---
>  gcc/genopinit.cc   | 5 ++---
>  gcc/gensupport.cc  | 2 +-
>  gcc/optabs-query.h | 4 ++--
>  3 files changed, 5 insertions(+), 6 deletions(-)
> 
> diff --git a/gcc/genopinit.cc b/gcc/genopinit.cc
> index 6bd8858a1d9..2a841006884 100644
> --- a/gcc/genopinit.cc
> +++ b/gcc/genopinit.cc
> @@ -182,8 +182,7 @@ main (int argc, const char **argv)
>  
>    progname = "genopinit";
>  
> -  if (NUM_OPTABS > 0xffff
> -    || MAX_MACHINE_MODE >= ((1 << MACHINE_MODE_BITSIZE) - 1))
> +  if (NUM_OPTABS > 0xfff || NUM_MACHINE_MODES > 0x3ff)
>      fatal ("genopinit range assumptions invalid");
>  
>    if (!init_rtx_reader_args_cb (argc, argv, handle_arg))
> @@ -439,7 +438,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 scode = (op << 20) | 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..959d1d9c83c 100644
> --- a/gcc/gensupport.cc
> +++ b/gcc/gensupport.cc
> @@ -3806,7 +3806,7 @@ 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 = (p->op << 20) | (p->m2 << 10) | p->m1;
>  	  return true;
>  	}
>      }
> diff --git a/gcc/optabs-query.h b/gcc/optabs-query.h
> index 043e9791bc1..920eb6a1b67 100644
> --- a/gcc/optabs-query.h
> +++ b/gcc/optabs-query.h
> @@ -37,7 +37,7 @@ convert_optab_p (optab op)
>  inline enum insn_code
>  optab_handler (optab op, machine_mode mode)
>  {
> -  unsigned scode = (op << 16) | mode;
> +  unsigned scode = (op << 20) | mode;
>    gcc_assert (op > LAST_CONV_OPTAB);
>    return raw_optab_handler (scode);
>  }
> @@ -50,7 +50,7 @@ inline enum insn_code
>  convert_optab_handler (convert_optab op, machine_mode to_mode,
>  		       machine_mode from_mode)
>  {
> -  unsigned scode = (op << 16) | (from_mode << 8) | to_mode;
> +  unsigned scode = (op << 20) | (from_mode << 10) | to_mode;
>    gcc_assert (convert_optab_p (op));
>    return raw_optab_handler (scode);
>  }
> 

-- 
Richard Biener <rguenther@suse.de>
SUSE Software Solutions Germany GmbH, Frankenstrasse 146, 90461 Nuernberg,
Germany; GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman;
HRB 36809 (AG Nuernberg)