| Message ID | 6374e0bb-f7ce-45e9-9fc2-48438fb97664@linux.ibm.com |
|---|---|
| State | New |
| Headers | show |
| Series | rs6000: Fix ICE in rs6000_init_builtins when compiling with -mcpu=440 [PR99279] | expand |
On Thu, Feb 25, 2021 at 8:05 PM Peter Bergner <bergner@linux.ibm.com> wrote: > > The initialization of compat builtins assumes the builtin we are creating > a compatible builtin for exists and ICEs if it doesn't. However, there are > valid reasons why some builtins are disabled for a particular compile. > In this case, the MMA builtins are disabled for -mcpu=440 (and other cpus), > so instead of ICEing, we should just skip adding the MMA compat builtin. > > This passed bootstrap and regtesting on powerpc64-linux, with running the > testsuite in both 32-bit and 64-bit modes, with no regressions. > Ok for mainline? Okay. Thanks, David > > The compat builtin patch was approved for backporting to GCC10, so we'll > need this fix to go along with it. > > Peter > > > 2021-02-25 Peter Bergner <bergner@linux.ibm.com> > > gcc/ > PR target/99279 > * config/rs6000/rs6000-call.c (rs6000_init_builtins): Replace assert > with an "if" test. > > diff --git a/gcc/config/rs6000/rs6000-call.c b/gcc/config/rs6000/rs6000-call.c > index d2bd03e..f567625 100644 > --- a/gcc/config/rs6000/rs6000-call.c > +++ b/gcc/config/rs6000/rs6000-call.c > @@ -13468,9 +13468,9 @@ rs6000_init_builtins (void) > for (i = 0; i < ARRAY_SIZE (bdesc_compat); i++, d++) > { > tree decl = rs6000_builtin_decls[(int)d->code]; > - gcc_assert (decl != NULL); > - add_builtin_function (d->name, TREE_TYPE (decl), (int)d->code, > - BUILT_IN_MD, NULL, NULL_TREE); > + if (decl != NULL) > + add_builtin_function (d->name, TREE_TYPE (decl), (int)d->code, > + BUILT_IN_MD, NULL, NULL_TREE); > } > } >
On Thu, Feb 25, 2021 at 07:05:26PM -0600, Peter Bergner wrote: > The initialization of compat builtins assumes the builtin we are creating > a compatible builtin for exists and ICEs if it doesn't. However, there are > valid reasons why some builtins are disabled for a particular compile. > In this case, the MMA builtins are disabled for -mcpu=440 (and other cpus), > so instead of ICEing, we should just skip adding the MMA compat builtin. > > This passed bootstrap and regtesting on powerpc64-linux, with running the > testsuite in both 32-bit and 64-bit modes, with no regressions. > Ok for mainline? Okay, thank you! > The compat builtin patch was approved for backporting to GCC10, so we'll > need this fix to go along with it. Okay for that as well of course. Segher > PR target/99279 > * config/rs6000/rs6000-call.c (rs6000_init_builtins): Replace assert > with an "if" test.
On 2/25/21 7:08 PM, David Edelsohn wrote: > On Thu, Feb 25, 2021 at 8:05 PM Peter Bergner <bergner@linux.ibm.com> wrote: >> >> The initialization of compat builtins assumes the builtin we are creating >> a compatible builtin for exists and ICEs if it doesn't. However, there are >> valid reasons why some builtins are disabled for a particular compile. >> In this case, the MMA builtins are disabled for -mcpu=440 (and other cpus), >> so instead of ICEing, we should just skip adding the MMA compat builtin. >> >> This passed bootstrap and regtesting on powerpc64-linux, with running the >> testsuite in both 32-bit and 64-bit modes, with no regressions. >> Ok for mainline? > > Okay. Thanks, pushed. On 2/25/21 7:09 PM, Segher Boessenkool wrote: > On Thu, Feb 25, 2021 at 07:05:26PM -0600, Peter Bergner wrote: >> The compat builtin patch was approved for backporting to GCC10, so we'll >> need this fix to go along with it. > > Okay for that as well of course. Thanks, I'll give this a day or two and then push the two to gcc10. Do you want them committed separately or squashed into one commit since the 2nd patch fixes the issue in the first patch? Peter
On Thu, Feb 25, 2021 at 09:40:58PM -0600, Peter Bergner wrote: > On 2/25/21 7:09 PM, Segher Boessenkool wrote: > > On Thu, Feb 25, 2021 at 07:05:26PM -0600, Peter Bergner wrote: > >> The compat builtin patch was approved for backporting to GCC10, so we'll > >> need this fix to go along with it. > > > > Okay for that as well of course. > > Thanks, I'll give this a day or two and then push the two to gcc10. > Do you want them committed separately or squashed into one commit > since the 2nd patch fixes the issue in the first patch? Traditionally, we do one commit for such backports, with the original commit messages in it. I do not know how well this works with the server-side changelog stuff. Maybe you can cherry-pick (with -x) and squash the two patches? It isn't a super big problem if you backport the possible bisect break as well (in reality, will it ever be an issue for anyone?), but it is better to avoid it of course. Segher
On 2/25/21 9:40 PM, Peter Bergner wrote: > On 2/25/21 7:09 PM, Segher Boessenkool wrote: >> On Thu, Feb 25, 2021 at 07:05:26PM -0600, Peter Bergner wrote: >>> The compat builtin patch was approved for backporting to GCC10, so we'll >>> need this fix to go along with it. >> >> Okay for that as well of course. > > Thanks, I'll give this a day or two and then push the two to gcc10. > Do you want them committed separately or squashed into one commit > since the 2nd patch fixes the issue in the first patch? I pushed both backport commits, so fixed everywhere. Thanks. Peter
diff --git a/gcc/config/rs6000/rs6000-call.c b/gcc/config/rs6000/rs6000-call.c index d2bd03e..f567625 100644 --- a/gcc/config/rs6000/rs6000-call.c +++ b/gcc/config/rs6000/rs6000-call.c @@ -13468,9 +13468,9 @@ rs6000_init_builtins (void) for (i = 0; i < ARRAY_SIZE (bdesc_compat); i++, d++) { tree decl = rs6000_builtin_decls[(int)d->code]; - gcc_assert (decl != NULL); - add_builtin_function (d->name, TREE_TYPE (decl), (int)d->code, - BUILT_IN_MD, NULL, NULL_TREE); + if (decl != NULL) + add_builtin_function (d->name, TREE_TYPE (decl), (int)d->code, + BUILT_IN_MD, NULL, NULL_TREE); } }