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

[钟居哲 <juzhe.zhong@rivai.ai>]

[-- 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