[PATCH] libgccjit: Fix get_size of size_t

[PATCH] libgccjit: Fix get_size of size_t

[Antoni Boucher]

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

Hi.
This patch fixes getting the size of size_t (bug 112910).

There's one issue with this patch: like every other feature that checks
for target-specific stuff, it requires a compilation before actually
fetching the size of the type.
Which means that getting the size before a compilation might be wrong
(and I actually believe is wrong on x86-64).

I was wondering if we should always implicitely do the first
compilation to gather the correct info: this would fix this issue and
all the others that we have due to that.
I'm not sure what would be the performance implication.

Another solution that I have been thinking about for a while now would
be to have another frontend libgccaot (I don't like that name), which
is like libgccjit but removes the JIT part so that we get access to the
target stuff directly and would remove the need for having a seperation
between recording and playback as far as I understand.
That's a long-term solution, but I wanted to share the idea now and
gather your thoughts on that.

Thanks for the review.

[-- Attachment #2: 0001-libgccjit-Fix-get_size-of-size_t.patch --]
[-- Type: text/x-patch, Size: 1911 bytes --]

From 37d25e55a0c79893c7ea7f4cb9f0842b8a9b4906 Mon Sep 17 00:00:00 2001
From: Antoni Boucher <bouanto@zoho.com>
Date: Fri, 3 Nov 2023 17:49:18 -0400
Subject: [PATCH] libgccjit: Fix get_size of size_t

gcc/jit/ChangeLog:
	PR jit/112910
	* jit-recording.cc (recording::memento_of_get_type::get_size):
	Correctly compute the size of size_t.
---
 gcc/jit/jit-recording.cc | 29 ++++++++++++++++++++++++++++-
 1 file changed, 28 insertions(+), 1 deletion(-)

diff --git a/gcc/jit/jit-recording.cc b/gcc/jit/jit-recording.cc
index 9b5b8005ebe..ea1f76d4415 100644
--- a/gcc/jit/jit-recording.cc
+++ b/gcc/jit/jit-recording.cc
@@ -2392,7 +2392,34 @@ recording::memento_of_get_type::get_size ()
       size = LONG_DOUBLE_TYPE_SIZE;
       break;
     case GCC_JIT_TYPE_SIZE_T:
-      size = MAX_BITS_PER_WORD;
+      /* Compare with tree.cc's build_common_tree_nodes.  */
+      if (strcmp (SIZE_TYPE, "unsigned int") == 0)
+        size = INT_TYPE_SIZE;
+      else if (strcmp (SIZE_TYPE, "long unsigned int") == 0)
+        size = LONG_TYPE_SIZE;
+      else if (strcmp (SIZE_TYPE, "long long unsigned int") == 0)
+        size = LONG_LONG_TYPE_SIZE;
+      else if (strcmp (SIZE_TYPE, "short unsigned int") == 0)
+        size = SHORT_TYPE_SIZE;
+      else
+      {
+        int i;
+
+        for (i = 0; i < NUM_INT_N_ENTS; i++)
+          if (int_n_enabled_p[i])
+          {
+            fprintf (stderr, "%d\n", i);
+            char name[50], altname[50];
+            sprintf (name, "__int%d unsigned", int_n_data[i].bitsize);
+            sprintf (altname, "__int%d__ unsigned", int_n_data[i].bitsize);
+
+            if (strcmp (name, SIZE_TYPE) == 0 || strcmp (altname, SIZE_TYPE) == 0)
+            {
+              return int_n_data[i].bitsize / BITS_PER_UNIT;
+            }
+          }
+        gcc_unreachable ();
+      }
       break;
     default:
       /* As this function is called by
-- 
2.43.0

Re: [PATCH] libgccjit: Fix get_size of size_t

[David Malcolm]

On Thu, 2023-12-07 at 17:26 -0500, Antoni Boucher wrote:
> Hi.
> This patch fixes getting the size of size_t (bug 112910).
> 
> There's one issue with this patch: like every other feature that
> checks
> for target-specific stuff, it requires a compilation before actually
> fetching the size of the type.
> Which means that getting the size before a compilation might be wrong
> (and I actually believe is wrong on x86-64).
> 
> I was wondering if we should always implicitely do the first
> compilation to gather the correct info: this would fix this issue and
> all the others that we have due to that.
> I'm not sure what would be the performance implication.

Maybe introduce a new class target_info which contains all the
information we might want to find via a compilation, and have the top-
level recording::context have a pointer to it, which starts as nullptr,
but can be populated on-demand the first time something needs it?

> 
> Another solution that I have been thinking about for a while now
> would
> be to have another frontend libgccaot (I don't like that name), which
> is like libgccjit but removes the JIT part so that we get access to
> the
> target stuff directly and would remove the need for having a
> seperation
> between recording and playback as far as I understand.
> That's a long-term solution, but I wanted to share the idea now and
> gather your thoughts on that.

FWIW the initial version of libgccjit didn't have a split between
recording and playback; instead the client code had to pass in a
callback to call into the various API functions (creating tree nodes).
See:
https://gcc.gnu.org/legacy-ml/gcc-patches/2013-10/msg00228.html

Dave

Re: [PATCH] libgccjit: Fix get_size of size_t

[Antoni Boucher]

On Thu, 2023-12-07 at 19:57 -0500, David Malcolm wrote:
> On Thu, 2023-12-07 at 17:26 -0500, Antoni Boucher wrote:
> > Hi.
> > This patch fixes getting the size of size_t (bug 112910).
> > 
> > There's one issue with this patch: like every other feature that
> > checks
> > for target-specific stuff, it requires a compilation before
> > actually
> > fetching the size of the type.
> > Which means that getting the size before a compilation might be
> > wrong
> > (and I actually believe is wrong on x86-64).
> > 
> > I was wondering if we should always implicitely do the first
> > compilation to gather the correct info: this would fix this issue
> > and
> > all the others that we have due to that.
> > I'm not sure what would be the performance implication.
> 
> Maybe introduce a new class target_info which contains all the
> information we might want to find via a compilation, and have the
> top-
> level recording::context have a pointer to it, which starts as
> nullptr,
> but can be populated on-demand the first time something needs it?

That would mean that we'll need to populate it for every top-level
context, right? Would the idea be that we should then use child
contexts to have the proper information filled?
If so, how is this different than just compiling two contexts like what
I currently do?
This would also mean that we'll do an implicit compilation whenever we
use an API that needs this info, right? Wouldn't that be unexpected?

Thanks for the idea.

> 
> > 
> > Another solution that I have been thinking about for a while now
> > would
> > be to have another frontend libgccaot (I don't like that name),
> > which
> > is like libgccjit but removes the JIT part so that we get access to
> > the
> > target stuff directly and would remove the need for having a
> > seperation
> > between recording and playback as far as I understand.
> > That's a long-term solution, but I wanted to share the idea now and
> > gather your thoughts on that.
> 
> FWIW the initial version of libgccjit didn't have a split between
> recording and playback; instead the client code had to pass in a
> callback to call into the various API functions (creating tree
> nodes).
> See:
> https://gcc.gnu.org/legacy-ml/gcc-patches/2013-10/msg00228.html
> 
> Dave
>