Re: [pushed: r15-7126] jit: fix startup on aarch64
On Wed, 2025-01-22 at 08:47 -0500, Antoni Boucher wrote:
> Hi David.
Hi Antoni
I went ahead and pushed this patch since without it even simple unit
tests like test-factorial.c were failing for me on aarch64; in
particular, the build of emacs was failing in Fedora's mass rebuild
with gcc 15, and the patch seems to be a minimal way to fix this.
> I had a patch for this here:
> https://github.com/antoyo/libgccjit/pull/20
Sorry for this getting stuck. I see you marked that as "waiting for
review", but it looks like the patch there hasn't changed since my last
review within the github web UI; was that to signify that you were
hoping for an answer to the questions you asked on how to better
implement the fix?
>
> The fact that you removed the debug_tree (and abort) will make it
> harder
> to figure out what the missing types to handle are.
> This will also probably make it hard for people to understand why
> they
> get a type error when calling a builtin function with an unsupported
> type.
> And as you can see in my PR, at least a few types were missing that
> you
> didn't add in your patch.
>
> Do you have a better solution for this?
>
> I just thought about this potential solution: perhaps if we get an
> unsupported type, we could add the builtin to an array instead of the
> hashmap: this way, we could tell the user that this builtin is not
> currently supported.
> What are your thoughts on this?
I'll take another look at PR 117886 now and see if I can implement
something.
Dave
>
> Thanks.
>
> Le 2025-01-22 à 08 h 38, David Malcolm a écrit :
> > libgccjit fails on startup on aarch64 (and probably other archs).
> >
> > The issues are that
> >
> > (a) within jit_langhook_init the call to
> > targetm.init_builtins can use types that aren't representable
> > via jit::recording::type, and
> >
> > (b) targetm.init_builtins can call lang_hooks.decls.pushdecl, which
> > although a no-op for libgccjit has a gcc_unreachable.
> >
> > Fixed thusly.
> >
> > Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu.
> > Pushed to trunk as r15-7126-g27470f9a818538.
> >
> > gcc/jit/ChangeLog:
> > * dummy-frontend.cc (tree_type_to_jit_type): For
> > POINTER_TYPE,
> > bail out if the inner call to tree_type_to_jit_type fails.
> > Don't abort on unknown types.
> > (jit_langhook_pushdecl): Replace gcc_unreachable with
> > return of
> > NULL_TREE.
> >
> > Signed-off-by: David Malcolm
> > ---
> > gcc/jit/dummy-frontend.cc | 8 +++-
> > 1 file changed, 3 insertions(+), 5 deletions(-)
> >
> > diff --git a/gcc/jit/dummy-frontend.cc b/gcc/jit/dummy-frontend.cc
> > index 574851696311..1d0080d6fecb 100644
> > --- a/gcc/jit/dummy-frontend.cc
> > +++ b/gcc/jit/dummy-frontend.cc
> > @@ -1278,6 +1278,8 @@ recording::type* tree_type_to_jit_type (tree
> > type)
> > {
> > tree inner_type = TREE_TYPE (type);
> > recording::type* element_type = tree_type_to_jit_type
> > (inner_type);
> > + if (!element_type)
> > + return nullptr;
> > return element_type->get_pointer ();
> > }
> > else
> > @@ -1299,10 +1301,6 @@ recording::type* tree_type_to_jit_type (tree
> > type)
> > }
> > }
> > }
> > -
> > - fprintf (stderr, "Unknown type:\n");
> > - debug_tree (type);
> > - abort ();
> > }
> >
> > return NULL;
> > @@ -1372,7 +1370,7 @@ jit_langhook_global_bindings_p (void)
> > static tree
> > jit_langhook_pushdecl (tree decl ATTRIBUTE_UNUSED)
> > {
> > - gcc_unreachable ();
> > + return NULL_TREE;
> > }
> >
> > static tree
>
Re: [pushed: r15-7126] jit: fix startup on aarch64
Le 2025-01-23 à 13 h 13, David Malcolm a écrit :
On Wed, 2025-01-22 at 08:47 -0500, Antoni Boucher wrote:
Hi David.
Hi Antoni
I went ahead and pushed this patch since without it even simple unit
tests like test-factorial.c were failing for me on aarch64; in
particular, the build of emacs was failing in Fedora's mass rebuild
with gcc 15, and the patch seems to be a minimal way to fix this.
I had a patch for this here:
https://github.com/antoyo/libgccjit/pull/20
Sorry for this getting stuck. I see you marked that as "waiting for
review", but it looks like the patch there hasn't changed since my last
review within the github web UI; was that to signify that you were
hoping for an answer to the questions you asked on how to better
implement the fix?
Yes, perhaps I should have added the label "waiting for info", sorry.
Btw, could you please answer to this message to give your thoughts on
your workflow and usage of labels:
https://sourceware.org/pipermail/forge/2024q4/78.html
The fact that you removed the debug_tree (and abort) will make it
harder
to figure out what the missing types to handle are.
This will also probably make it hard for people to understand why
they
get a type error when calling a builtin function with an unsupported
type.
And as you can see in my PR, at least a few types were missing that
you
didn't add in your patch.
Do you have a better solution for this?
I just thought about this potential solution: perhaps if we get an
unsupported type, we could add the builtin to an array instead of the
hashmap: this way, we could tell the user that this builtin is not
currently supported.
What are your thoughts on this?
I'll take another look at PR 117886 now and see if I can implement
something.
Thanks!
Dave
Thanks.
Le 2025-01-22 à 08 h 38, David Malcolm a écrit :
libgccjit fails on startup on aarch64 (and probably other archs).
The issues are that
(a) within jit_langhook_init the call to
targetm.init_builtins can use types that aren't representable
via jit::recording::type, and
(b) targetm.init_builtins can call lang_hooks.decls.pushdecl, which
although a no-op for libgccjit has a gcc_unreachable.
Fixed thusly.
Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu.
Pushed to trunk as r15-7126-g27470f9a818538.
gcc/jit/ChangeLog:
* dummy-frontend.cc (tree_type_to_jit_type): For
POINTER_TYPE,
bail out if the inner call to tree_type_to_jit_type fails.
Don't abort on unknown types.
(jit_langhook_pushdecl): Replace gcc_unreachable with
return of
NULL_TREE.
Signed-off-by: David Malcolm
---
gcc/jit/dummy-frontend.cc | 8 +++-
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/gcc/jit/dummy-frontend.cc b/gcc/jit/dummy-frontend.cc
index 574851696311..1d0080d6fecb 100644
--- a/gcc/jit/dummy-frontend.cc
+++ b/gcc/jit/dummy-frontend.cc
@@ -1278,6 +1278,8 @@ recording::type* tree_type_to_jit_type (tree
type)
{
tree inner_type = TREE_TYPE (type);
recording::type* element_type = tree_type_to_jit_type
(inner_type);
+ if (!element_type)
+ return nullptr;
return element_type->get_pointer ();
}
else
@@ -1299,10 +1301,6 @@ recording::type* tree_type_to_jit_type (tree
type)
}
}
}
-
- fprintf (stderr, "Unknown type:\n");
- debug_tree (type);
- abort ();
}
return NULL;
@@ -1372,7 +1370,7 @@ jit_langhook_global_bindings_p (void)
static tree
jit_langhook_pushdecl (tree decl ATTRIBUTE_UNUSED)
{
- gcc_unreachable ();
+ return NULL_TREE;
}
static tree
