| Message ID | alpine.LSU.2.20.1804261407240.24704@zhemvz.fhfr.qr |
|---|---|
| State | New |
| Headers | show |
| Series | Fix aarch64 ILP32 ICE with vaarg gimplified code | expand |
On 26 April 2018 at 14:09, Richard Biener <rguenther@suse.de> wrote: > > Seen by Christophe Lyon, verified with a cross that this fixes the > issue. > > aarch64 people, can you please test & commit this? > As I have just written in bugzilla, this patch avoids an ICE when compiling newlib's sysopen.i, but I still see a similar crash when compiling vfprintf.i. Thanks, Christophe > Thanks, > Richard. > > 2018-04-26 Richard Biener <rguenther@suse.de> > > * config/aarch64/aarch64.c: Simplify ap.__stack advance and > fix for ILP32. > > Index: gcc/config/aarch64/aarch64.c > =================================================================== > --- gcc/config/aarch64/aarch64.c (revision 259669) > +++ gcc/config/aarch64/aarch64.c (working copy) > @@ -12278,12 +12278,9 @@ aarch64_gimplify_va_arg_expr (tree valis > else > roundup = NULL; > /* Advance ap.__stack */ > - t = fold_convert (intDI_type_node, arg); > - t = build2 (PLUS_EXPR, TREE_TYPE (t), t, > - build_int_cst (TREE_TYPE (t), size + 7)); > + t = fold_build_pointer_plus_hwi (arg, size + 7); > t = build2 (BIT_AND_EXPR, TREE_TYPE (t), t, > build_int_cst (TREE_TYPE (t), -8)); > - t = fold_convert (TREE_TYPE (arg), t); > t = build2 (MODIFY_EXPR, TREE_TYPE (stack), unshare_expr (stack), t); > /* String up roundup and advance. */ > if (roundup)
On Thu, 26 Apr 2018, Christophe Lyon wrote: > On 26 April 2018 at 14:09, Richard Biener <rguenther@suse.de> wrote: > > > > Seen by Christophe Lyon, verified with a cross that this fixes the > > issue. > > > > aarch64 people, can you please test & commit this? > > > > As I have just written in bugzilla, this patch avoids an ICE when > compiling newlib's sysopen.i, > but I still see a similar crash when compiling vfprintf.i. There's a similar case still left. Complete patch: Index: gcc/config/aarch64/aarch64.c =================================================================== --- gcc/config/aarch64/aarch64.c (revision 259669) +++ gcc/config/aarch64/aarch64.c (working copy) @@ -12267,23 +12267,17 @@ aarch64_gimplify_va_arg_expr (tree valis if (align > 8) { /* if (alignof(type) > 8) (arg = arg + 15) & -16; */ - t = fold_convert (intDI_type_node, arg); - t = build2 (PLUS_EXPR, TREE_TYPE (t), t, - build_int_cst (TREE_TYPE (t), 15)); + t = fold_build_pointer_plus_hwi (arg, 15); t = build2 (BIT_AND_EXPR, TREE_TYPE (t), t, build_int_cst (TREE_TYPE (t), -16)); - t = fold_convert (TREE_TYPE (arg), t); roundup = build2 (MODIFY_EXPR, TREE_TYPE (arg), arg, t); } else roundup = NULL; /* Advance ap.__stack */ - t = fold_convert (intDI_type_node, arg); - t = build2 (PLUS_EXPR, TREE_TYPE (t), t, - build_int_cst (TREE_TYPE (t), size + 7)); + t = fold_build_pointer_plus_hwi (arg, size + 7); t = build2 (BIT_AND_EXPR, TREE_TYPE (t), t, build_int_cst (TREE_TYPE (t), -8)); - t = fold_convert (TREE_TYPE (arg), t); t = build2 (MODIFY_EXPR, TREE_TYPE (stack), unshare_expr (stack), t); /* String up roundup and advance. */ if (roundup) > Thanks, > > Christophe > > > Thanks, > > Richard. > > > > 2018-04-26 Richard Biener <rguenther@suse.de> > > > > * config/aarch64/aarch64.c: Simplify ap.__stack advance and > > fix for ILP32. > > > > Index: gcc/config/aarch64/aarch64.c > > =================================================================== > > --- gcc/config/aarch64/aarch64.c (revision 259669) > > +++ gcc/config/aarch64/aarch64.c (working copy) > > @@ -12278,12 +12278,9 @@ aarch64_gimplify_va_arg_expr (tree valis > > else > > roundup = NULL; > > /* Advance ap.__stack */ > > - t = fold_convert (intDI_type_node, arg); > > - t = build2 (PLUS_EXPR, TREE_TYPE (t), t, > > - build_int_cst (TREE_TYPE (t), size + 7)); > > + t = fold_build_pointer_plus_hwi (arg, size + 7); > > t = build2 (BIT_AND_EXPR, TREE_TYPE (t), t, > > build_int_cst (TREE_TYPE (t), -8)); > > - t = fold_convert (TREE_TYPE (arg), t); > > t = build2 (MODIFY_EXPR, TREE_TYPE (stack), unshare_expr (stack), t); > > /* String up roundup and advance. */ > > if (roundup) > >
On 26 April 2018 at 15:41, Richard Biener <rguenther@suse.de> wrote: > On Thu, 26 Apr 2018, Christophe Lyon wrote: > >> On 26 April 2018 at 14:09, Richard Biener <rguenther@suse.de> wrote: >> > >> > Seen by Christophe Lyon, verified with a cross that this fixes the >> > issue. >> > >> > aarch64 people, can you please test & commit this? >> > >> >> As I have just written in bugzilla, this patch avoids an ICE when >> compiling newlib's sysopen.i, >> but I still see a similar crash when compiling vfprintf.i. > > There's a similar case still left. Complete patch: > With this version, the toolchain build completes, but I didn't run make check, nor tried aarch64-linux-gnu. > Index: gcc/config/aarch64/aarch64.c > =================================================================== > --- gcc/config/aarch64/aarch64.c (revision 259669) > +++ gcc/config/aarch64/aarch64.c (working copy) > @@ -12267,23 +12267,17 @@ aarch64_gimplify_va_arg_expr (tree valis > if (align > 8) > { > /* if (alignof(type) > 8) (arg = arg + 15) & -16; */ > - t = fold_convert (intDI_type_node, arg); > - t = build2 (PLUS_EXPR, TREE_TYPE (t), t, > - build_int_cst (TREE_TYPE (t), 15)); > + t = fold_build_pointer_plus_hwi (arg, 15); > t = build2 (BIT_AND_EXPR, TREE_TYPE (t), t, > build_int_cst (TREE_TYPE (t), -16)); > - t = fold_convert (TREE_TYPE (arg), t); > roundup = build2 (MODIFY_EXPR, TREE_TYPE (arg), arg, t); > } > else > roundup = NULL; > /* Advance ap.__stack */ > - t = fold_convert (intDI_type_node, arg); > - t = build2 (PLUS_EXPR, TREE_TYPE (t), t, > - build_int_cst (TREE_TYPE (t), size + 7)); > + t = fold_build_pointer_plus_hwi (arg, size + 7); > t = build2 (BIT_AND_EXPR, TREE_TYPE (t), t, > build_int_cst (TREE_TYPE (t), -8)); > - t = fold_convert (TREE_TYPE (arg), t); > t = build2 (MODIFY_EXPR, TREE_TYPE (stack), unshare_expr (stack), t); > /* String up roundup and advance. */ > if (roundup) > > >> Thanks, >> >> Christophe >> >> > Thanks, >> > Richard. >> > >> > 2018-04-26 Richard Biener <rguenther@suse.de> >> > >> > * config/aarch64/aarch64.c: Simplify ap.__stack advance and >> > fix for ILP32. >> > >> > Index: gcc/config/aarch64/aarch64.c >> > =================================================================== >> > --- gcc/config/aarch64/aarch64.c (revision 259669) >> > +++ gcc/config/aarch64/aarch64.c (working copy) >> > @@ -12278,12 +12278,9 @@ aarch64_gimplify_va_arg_expr (tree valis >> > else >> > roundup = NULL; >> > /* Advance ap.__stack */ >> > - t = fold_convert (intDI_type_node, arg); >> > - t = build2 (PLUS_EXPR, TREE_TYPE (t), t, >> > - build_int_cst (TREE_TYPE (t), size + 7)); >> > + t = fold_build_pointer_plus_hwi (arg, size + 7); >> > t = build2 (BIT_AND_EXPR, TREE_TYPE (t), t, >> > build_int_cst (TREE_TYPE (t), -8)); >> > - t = fold_convert (TREE_TYPE (arg), t); >> > t = build2 (MODIFY_EXPR, TREE_TYPE (stack), unshare_expr (stack), t); >> > /* String up roundup and advance. */ >> > if (roundup) >> >> > > -- > Richard Biener <rguenther@suse.de> > SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg)
On 26/04/18 15:04, Christophe Lyon wrote: > On 26 April 2018 at 15:41, Richard Biener <rguenther@suse.de> wrote: > > On Thu, 26 Apr 2018, Christophe Lyon wrote: > > > >> On 26 April 2018 at 14:09, Richard Biener <rguenther@suse.de> wrote: > >> > > >> > Seen by Christophe Lyon, verified with a cross that this fixes the > >> > issue. > >> > > >> > aarch64 people, can you please test & commit this? > >> > > >> > >> As I have just written in bugzilla, this patch avoids an ICE when > >> compiling newlib's sysopen.i, > >> but I still see a similar crash when compiling vfprintf.i. > > > > There's a similar case still left. Complete patch: > > > With this version, the toolchain build completes, but I didn't run > make check, nor tried aarch64-linux-gnu. > A bootstrap and test on aarch64-none-linux-gnu shows no problems. I don't know if there was a rationale for having the conversion to intDI_type_node. Thanks, Kyrill > > Index: gcc/config/aarch64/aarch64.c > > =================================================================== > > --- gcc/config/aarch64/aarch64.c (revision 259669) > > +++ gcc/config/aarch64/aarch64.c (working copy) > > @@ -12267,23 +12267,17 @@ aarch64_gimplify_va_arg_expr (tree valis > > if (align > 8) > > { > > /* if (alignof(type) > 8) (arg = arg + 15) & -16; */ > > - t = fold_convert (intDI_type_node, arg); > > - t = build2 (PLUS_EXPR, TREE_TYPE (t), t, > > - build_int_cst (TREE_TYPE (t), 15)); > > + t = fold_build_pointer_plus_hwi (arg, 15); > > t = build2 (BIT_AND_EXPR, TREE_TYPE (t), t, > > build_int_cst (TREE_TYPE (t), -16)); > > - t = fold_convert (TREE_TYPE (arg), t); > > roundup = build2 (MODIFY_EXPR, TREE_TYPE (arg), arg, t); > > } > > else > > roundup = NULL; > > /* Advance ap.__stack */ > > - t = fold_convert (intDI_type_node, arg); > > - t = build2 (PLUS_EXPR, TREE_TYPE (t), t, > > - build_int_cst (TREE_TYPE (t), size + 7)); > > + t = fold_build_pointer_plus_hwi (arg, size + 7); > > t = build2 (BIT_AND_EXPR, TREE_TYPE (t), t, > > build_int_cst (TREE_TYPE (t), -8)); > > - t = fold_convert (TREE_TYPE (arg), t); > > t = build2 (MODIFY_EXPR, TREE_TYPE (stack), unshare_expr (stack), t); > > /* String up roundup and advance. */ > > if (roundup) > > > > > >> Thanks, > >> > >> Christophe > >> > >> > Thanks, > >> > Richard. > >> > > >> > 2018-04-26 Richard Biener <rguenther@suse.de> > >> > > >> > * config/aarch64/aarch64.c: Simplify ap.__stack advance and > >> > fix for ILP32. > >> > > >> > Index: gcc/config/aarch64/aarch64.c > >> > =================================================================== > >> > --- gcc/config/aarch64/aarch64.c (revision 259669) > >> > +++ gcc/config/aarch64/aarch64.c (working copy) > >> > @@ -12278,12 +12278,9 @@ aarch64_gimplify_va_arg_expr (tree valis > >> > else > >> > roundup = NULL; > >> > /* Advance ap.__stack */ > >> > - t = fold_convert (intDI_type_node, arg); > >> > - t = build2 (PLUS_EXPR, TREE_TYPE (t), t, > >> > - build_int_cst (TREE_TYPE (t), size + 7)); > >> > + t = fold_build_pointer_plus_hwi (arg, size + 7); > >> > t = build2 (BIT_AND_EXPR, TREE_TYPE (t), t, > >> > build_int_cst (TREE_TYPE (t), -8)); > >> > - t = fold_convert (TREE_TYPE (arg), t); > >> > t = build2 (MODIFY_EXPR, TREE_TYPE (stack), unshare_expr (stack), t); > >> > /* String up roundup and advance. */ > >> > if (roundup) > >> > >> > > > > -- > > Richard Biener <rguenther@suse.de> > > SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg)
On Fri, 27 Apr 2018, Kyrill Tkachov wrote: > > On 26/04/18 15:04, Christophe Lyon wrote: > > On 26 April 2018 at 15:41, Richard Biener <rguenther@suse.de> wrote: > > > On Thu, 26 Apr 2018, Christophe Lyon wrote: > > > > > >> On 26 April 2018 at 14:09, Richard Biener <rguenther@suse.de> wrote: > > >> > > > >> > Seen by Christophe Lyon, verified with a cross that this fixes the > > >> > issue. > > >> > > > >> > aarch64 people, can you please test & commit this? > > >> > > > >> > > >> As I have just written in bugzilla, this patch avoids an ICE when > > >> compiling newlib's sysopen.i, > > >> but I still see a similar crash when compiling vfprintf.i. > > > > > > There's a similar case still left. Complete patch: > > > > > With this version, the toolchain build completes, but I didn't run > > make check, nor tried aarch64-linux-gnu. > > > > A bootstrap and test on aarch64-none-linux-gnu shows no problems. > I don't know if there was a rationale for having the conversion to > intDI_type_node. If that is somehow a required detail for SImode pointers then there needs to be a conversion from the pointer to uintSI_type_node before then extending to intDI_type_node. uintSI to match the setting of POINTERS_EXTEND_UNSIGNED for aarch64. Richard. > Thanks, > Kyrill > > > > Index: gcc/config/aarch64/aarch64.c > > > =================================================================== > > > --- gcc/config/aarch64/aarch64.c (revision 259669) > > > +++ gcc/config/aarch64/aarch64.c (working copy) > > > @@ -12267,23 +12267,17 @@ aarch64_gimplify_va_arg_expr (tree valis > > > if (align > 8) > > > { > > > /* if (alignof(type) > 8) (arg = arg + 15) & -16; */ > > > - t = fold_convert (intDI_type_node, arg); > > > - t = build2 (PLUS_EXPR, TREE_TYPE (t), t, > > > - build_int_cst (TREE_TYPE (t), 15)); > > > + t = fold_build_pointer_plus_hwi (arg, 15); > > > t = build2 (BIT_AND_EXPR, TREE_TYPE (t), t, > > > build_int_cst (TREE_TYPE (t), -16)); > > > - t = fold_convert (TREE_TYPE (arg), t); > > > roundup = build2 (MODIFY_EXPR, TREE_TYPE (arg), arg, t); > > > } > > > else > > > roundup = NULL; > > > /* Advance ap.__stack */ > > > - t = fold_convert (intDI_type_node, arg); > > > - t = build2 (PLUS_EXPR, TREE_TYPE (t), t, > > > - build_int_cst (TREE_TYPE (t), size + 7)); > > > + t = fold_build_pointer_plus_hwi (arg, size + 7); > > > t = build2 (BIT_AND_EXPR, TREE_TYPE (t), t, > > > build_int_cst (TREE_TYPE (t), -8)); > > > - t = fold_convert (TREE_TYPE (arg), t); > > > t = build2 (MODIFY_EXPR, TREE_TYPE (stack), unshare_expr (stack), t); > > > /* String up roundup and advance. */ > > > if (roundup) > > > > > > > > >> Thanks, > > >> > > >> Christophe > > >> > > >> > Thanks, > > >> > Richard. > > >> > > > >> > 2018-04-26 Richard Biener <rguenther@suse.de> > > >> > > > >> > * config/aarch64/aarch64.c: Simplify ap.__stack advance and > > >> > fix for ILP32. > > >> > > > >> > Index: gcc/config/aarch64/aarch64.c > > >> > =================================================================== > > >> > --- gcc/config/aarch64/aarch64.c (revision 259669) > > >> > +++ gcc/config/aarch64/aarch64.c (working copy) > > >> > @@ -12278,12 +12278,9 @@ aarch64_gimplify_va_arg_expr (tree valis > > >> > else > > >> > roundup = NULL; > > >> > /* Advance ap.__stack */ > > >> > - t = fold_convert (intDI_type_node, arg); > > >> > - t = build2 (PLUS_EXPR, TREE_TYPE (t), t, > > >> > - build_int_cst (TREE_TYPE (t), size + 7)); > > >> > + t = fold_build_pointer_plus_hwi (arg, size + 7); > > >> > t = build2 (BIT_AND_EXPR, TREE_TYPE (t), t, > > >> > build_int_cst (TREE_TYPE (t), -8)); > > >> > - t = fold_convert (TREE_TYPE (arg), t); > > >> > t = build2 (MODIFY_EXPR, TREE_TYPE (stack), unshare_expr (stack), > > t); > > >> > /* String up roundup and advance. */ > > >> > if (roundup) > > >> > > >> > > > > > > -- > > > Richard Biener <rguenther@suse.de> > > > SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB > > 21284 (AG Nuernberg) > >
On Fri, Apr 27, 2018 at 10:02:11AM +0100, Richard Biener wrote: > On Fri, 27 Apr 2018, Kyrill Tkachov wrote: > > > > > On 26/04/18 15:04, Christophe Lyon wrote: > > > On 26 April 2018 at 15:41, Richard Biener <rguenther@suse.de> wrote: > > > > On Thu, 26 Apr 2018, Christophe Lyon wrote: > > > > > > > >> On 26 April 2018 at 14:09, Richard Biener <rguenther@suse.de> wrote: > > > >> > > > > >> > Seen by Christophe Lyon, verified with a cross that this fixes the > > > >> > issue. > > > >> > > > > >> > aarch64 people, can you please test & commit this? > > > >> > > > > >> > > > >> As I have just written in bugzilla, this patch avoids an ICE when > > > >> compiling newlib's sysopen.i, > > > >> but I still see a similar crash when compiling vfprintf.i. > > > > > > > > There's a similar case still left. Complete patch: > > > > > > > With this version, the toolchain build completes, but I didn't run > > > make check, nor tried aarch64-linux-gnu. > > > > > > > A bootstrap and test on aarch64-none-linux-gnu shows no problems. > > I don't know if there was a rationale for having the conversion to > > intDI_type_node. > > If that is somehow a required detail for SImode pointers then > there needs to be a conversion from the pointer to uintSI_type_node > before then extending to intDI_type_node. uintSI to match the > setting of POINTERS_EXTEND_UNSIGNED for aarch64. Looks OK to me. Thanks for the patch and explanation. Kyrill, if you have it in tree you can commit it if that's what is wanted. Thanks, James
On 27/04/18 15:25, James Greenhalgh wrote: > On Fri, Apr 27, 2018 at 10:02:11AM +0100, Richard Biener wrote: > > On Fri, 27 Apr 2018, Kyrill Tkachov wrote: > > > > > > > > On 26/04/18 15:04, Christophe Lyon wrote: > > > > On 26 April 2018 at 15:41, Richard Biener <rguenther@suse.de> wrote: > > > > > On Thu, 26 Apr 2018, Christophe Lyon wrote: > > > > > > > > > >> On 26 April 2018 at 14:09, Richard Biener <rguenther@suse.de> wrote: > > > > >> > > > > > >> > Seen by Christophe Lyon, verified with a cross that this fixes the > > > > >> > issue. > > > > >> > > > > > >> > aarch64 people, can you please test & commit this? > > > > >> > > > > > >> > > > > >> As I have just written in bugzilla, this patch avoids an ICE when > > > > >> compiling newlib's sysopen.i, > > > > >> but I still see a similar crash when compiling vfprintf.i. > > > > > > > > > > There's a similar case still left. Complete patch: > > > > > > > > > With this version, the toolchain build completes, but I didn't run > > > > make check, nor tried aarch64-linux-gnu. > > > > > > > > > > A bootstrap and test on aarch64-none-linux-gnu shows no problems. > > > I don't know if there was a rationale for having the conversion to > > > intDI_type_node. > > > > If that is somehow a required detail for SImode pointers then > > there needs to be a conversion from the pointer to uintSI_type_node > > before then extending to intDI_type_node. uintSI to match the > > setting of POINTERS_EXTEND_UNSIGNED for aarch64. > > Looks OK to me. Thanks for the patch and explanation. Kyrill, if you > have it in tree you can commit it if that's what is wanted. > I've committed the patch with r259711. Thanks, Kyrill > Thanks, > James >
Index: gcc/config/aarch64/aarch64.c =================================================================== --- gcc/config/aarch64/aarch64.c (revision 259669) +++ gcc/config/aarch64/aarch64.c (working copy) @@ -12278,12 +12278,9 @@ aarch64_gimplify_va_arg_expr (tree valis else roundup = NULL; /* Advance ap.__stack */ - t = fold_convert (intDI_type_node, arg); - t = build2 (PLUS_EXPR, TREE_TYPE (t), t, - build_int_cst (TREE_TYPE (t), size + 7)); + t = fold_build_pointer_plus_hwi (arg, size + 7); t = build2 (BIT_AND_EXPR, TREE_TYPE (t), t, build_int_cst (TREE_TYPE (t), -8)); - t = fold_convert (TREE_TYPE (arg), t); t = build2 (MODIFY_EXPR, TREE_TYPE (stack), unshare_expr (stack), t); /* String up roundup and advance. */ if (roundup)