| Message ID | 20121103081427.GA5929@gmail.com |
|---|---|
| State | New |
| Headers | show |
On Sat, Nov 3, 2012 at 1:14 AM, H.J. Lu <hjl.tools@gmail.com> wrote: > Hi, > > The testcase shows -O -mx32 -maddress-mode=long -fPIC -S generates; > > x.i:22:37: internal compiler error: in plus_constant, at explow.c:88 > info[0x6ffffeff - dyn->d_tag + 12] = dyn; > > expand_expr_real_2 has > > /* No sense saving up arithmetic to be done > if it's all in the wrong mode to form part of an address. > And force_operand won't know whether to sign-extend or > zero-extend. */ > if ((modifier != EXPAND_SUM && modifier != EXPAND_INITIALIZER) > || mode != ptr_mode) > > With mode == SImode, Pmode == DImode and ptr_mode == SImode, we > generate wrong address. Instead of zero-extending address > 0xf7ffdd64, we sign-extend it to 0xfffffffff7ffdd64. This patch > checks Pmode instead of ptr_mode for address mode. It fixes the testcase > and generates a working x32 glibc. Is this patch correct? > > Thanks. > > > H.J. > --- > gcc/ > > 2012-11-03 H.J. Lu <hongjiu.lu@intel.com> > > PR middle-end/55142 > * expr.c (expand_expr_real_2): Check Pmode instead of ptr_mode > for wrong address mode. > > gcc/testsuite/ > > 2012-11-03 H.J. Lu <hongjiu.lu@intel.com> > > PR middle-end/55142 > * gcc.target/i386/pr55142.c: New file. > > diff --git a/gcc/expr.c b/gcc/expr.c > index 0ad3b57..1600380 100644 > --- a/gcc/expr.c > +++ b/gcc/expr.c > @@ -8290,7 +8290,7 @@ expand_expr_real_2 (sepops ops, rtx target, enum machine_mode tmode, > And force_operand won't know whether to sign-extend or > zero-extend. */ > if ((modifier != EXPAND_SUM && modifier != EXPAND_INITIALIZER) > - || mode != ptr_mode) > + || mode != Pmode) > { > expand_operands (treeop0, treeop1, > subtarget, &op0, &op1, EXPAND_NORMAL); > @@ -8333,7 +8333,7 @@ expand_expr_real_2 (sepops ops, rtx target, enum machine_mode tmode, > And force_operand won't know whether to sign-extend or > zero-extend. */ > if ((modifier != EXPAND_SUM && modifier != EXPAND_INITIALIZER) > - || mode != ptr_mode) > + || mode != Pmode) > goto binop; > > expand_operands (treeop0, treeop1, > diff --git a/gcc/testsuite/gcc.target/i386/pr55142.c b/gcc/testsuite/gcc.target/i386/pr55142.c > new file mode 100644 > index 0000000..c8a9625 > --- /dev/null > +++ b/gcc/testsuite/gcc.target/i386/pr55142.c > @@ -0,0 +1,34 @@ > +/* { dg-do compile { target { ! { ia32 } } } } */ > +/* { dg-require-effective-target fpic } */ > +/* { dg-options "-O2 -mx32 -maddress-mode=long -fpic" } */ > + > +typedef int int32_t; > +typedef unsigned int uint32_t; > +typedef int32_t Elf32_Sword; > +typedef struct > +{ > + Elf32_Sword d_tag; > +} Elf32_Dyn; > +struct link_map > +{ > + Elf32_Dyn *l_ld; > + Elf32_Dyn *l_info[34]; > +}; > +extern struct link_map _dl_rtld_map __attribute__ ((visibility ("hidden"))); > +static void elf_get_dynamic_info (struct link_map *l) > +{ > + Elf32_Dyn *dyn = l->l_ld; > + Elf32_Dyn **info; > + info = l->l_info; > + while (dyn->d_tag != 0) > + { > + if ((uint32_t) (0x6ffffeff - dyn->d_tag) < 11) > + info[0x6ffffeff - dyn->d_tag + 12] = dyn; > + ++dyn; > + } > +} > +void > +foo (void) > +{ > + elf_get_dynamic_info (&_dl_rtld_map); > +} Any comments? I'd like to get it fixed for 4.8. This patch only impacts targets with Pmode != ptr_mode. Richard, can you take a look at this for mips? Thanks.
"H.J. Lu" <hjl.tools@gmail.com> writes: > On Sat, Nov 3, 2012 at 1:14 AM, H.J. Lu <hjl.tools@gmail.com> wrote: >> Hi, >> >> The testcase shows -O -mx32 -maddress-mode=long -fPIC -S generates; >> >> x.i:22:37: internal compiler error: in plus_constant, at explow.c:88 >> info[0x6ffffeff - dyn->d_tag + 12] = dyn; >> >> expand_expr_real_2 has >> >> /* No sense saving up arithmetic to be done >> if it's all in the wrong mode to form part of an address. >> And force_operand won't know whether to sign-extend or >> zero-extend. */ >> if ((modifier != EXPAND_SUM && modifier != EXPAND_INITIALIZER) >> || mode != ptr_mode) >> >> With mode == SImode, Pmode == DImode and ptr_mode == SImode, we >> generate wrong address. Instead of zero-extending address >> 0xf7ffdd64, we sign-extend it to 0xfffffffff7ffdd64. This patch >> checks Pmode instead of ptr_mode for address mode. It fixes the testcase >> and generates a working x32 glibc. Is this patch correct? >> >> Thanks. >> >> >> H.J. >> --- >> gcc/ >> >> 2012-11-03 H.J. Lu <hongjiu.lu@intel.com> >> >> PR middle-end/55142 >> * expr.c (expand_expr_real_2): Check Pmode instead of ptr_mode >> for wrong address mode. >> >> gcc/testsuite/ >> >> 2012-11-03 H.J. Lu <hongjiu.lu@intel.com> >> >> PR middle-end/55142 >> * gcc.target/i386/pr55142.c: New file. >> >> diff --git a/gcc/expr.c b/gcc/expr.c >> index 0ad3b57..1600380 100644 >> --- a/gcc/expr.c >> +++ b/gcc/expr.c >> @@ -8290,7 +8290,7 @@ expand_expr_real_2 (sepops ops, rtx target, enum machine_mode tmode, >> And force_operand won't know whether to sign-extend or >> zero-extend. */ >> if ((modifier != EXPAND_SUM && modifier != EXPAND_INITIALIZER) >> - || mode != ptr_mode) >> + || mode != Pmode) >> { >> expand_operands (treeop0, treeop1, >> subtarget, &op0, &op1, EXPAND_NORMAL); >> @@ -8333,7 +8333,7 @@ expand_expr_real_2 (sepops ops, rtx target, enum machine_mode tmode, >> And force_operand won't know whether to sign-extend or >> zero-extend. */ >> if ((modifier != EXPAND_SUM && modifier != EXPAND_INITIALIZER) >> - || mode != ptr_mode) >> + || mode != Pmode) >> goto binop; >> >> expand_operands (treeop0, treeop1, >> diff --git a/gcc/testsuite/gcc.target/i386/pr55142.c b/gcc/testsuite/gcc.target/i386/pr55142.c >> new file mode 100644 >> index 0000000..c8a9625 >> --- /dev/null >> +++ b/gcc/testsuite/gcc.target/i386/pr55142.c >> @@ -0,0 +1,34 @@ >> +/* { dg-do compile { target { ! { ia32 } } } } */ >> +/* { dg-require-effective-target fpic } */ >> +/* { dg-options "-O2 -mx32 -maddress-mode=long -fpic" } */ >> + >> +typedef int int32_t; >> +typedef unsigned int uint32_t; >> +typedef int32_t Elf32_Sword; >> +typedef struct >> +{ >> + Elf32_Sword d_tag; >> +} Elf32_Dyn; >> +struct link_map >> +{ >> + Elf32_Dyn *l_ld; >> + Elf32_Dyn *l_info[34]; >> +}; >> +extern struct link_map _dl_rtld_map __attribute__ ((visibility ("hidden"))); >> +static void elf_get_dynamic_info (struct link_map *l) >> +{ >> + Elf32_Dyn *dyn = l->l_ld; >> + Elf32_Dyn **info; >> + info = l->l_info; >> + while (dyn->d_tag != 0) >> + { >> + if ((uint32_t) (0x6ffffeff - dyn->d_tag) < 11) >> + info[0x6ffffeff - dyn->d_tag + 12] = dyn; >> + ++dyn; >> + } >> +} >> +void >> +foo (void) >> +{ >> + elf_get_dynamic_info (&_dl_rtld_map); >> +} > > Any comments? I'd like to get it fixed for 4.8. I can't approve this anyway, but the existing ptr_mode sounds right for EXPAND_INTIALIZER. I assume it would be for EXPAND_SUM too, although I've always been a bit unsure what that means. > This patch only impacts targets with Pmode != ptr_mode. Richard, can > you take a look at this for mips? MIPS isn't affected FWIW. Having Pmode != ptr_mode was just a temporary experiment on the 3.4 rewrite branch. Richard
On Mon, Nov 5, 2012 at 3:06 PM, Richard Sandiford <rdsandiford@googlemail.com> wrote: > "H.J. Lu" <hjl.tools@gmail.com> writes: >> On Sat, Nov 3, 2012 at 1:14 AM, H.J. Lu <hjl.tools@gmail.com> wrote: >>> Hi, >>> >>> The testcase shows -O -mx32 -maddress-mode=long -fPIC -S generates; >>> >>> x.i:22:37: internal compiler error: in plus_constant, at explow.c:88 >>> info[0x6ffffeff - dyn->d_tag + 12] = dyn; >>> >>> expand_expr_real_2 has >>> >>> /* No sense saving up arithmetic to be done >>> if it's all in the wrong mode to form part of an address. >>> And force_operand won't know whether to sign-extend or >>> zero-extend. */ >>> if ((modifier != EXPAND_SUM && modifier != EXPAND_INITIALIZER) >>> || mode != ptr_mode) >>> >>> With mode == SImode, Pmode == DImode and ptr_mode == SImode, we >>> generate wrong address. Instead of zero-extending address >>> 0xf7ffdd64, we sign-extend it to 0xfffffffff7ffdd64. This patch >>> checks Pmode instead of ptr_mode for address mode. It fixes the testcase >>> and generates a working x32 glibc. Is this patch correct? >>> >>> Thanks. >>> >>> >>> H.J. >>> --- >>> gcc/ >>> >>> 2012-11-03 H.J. Lu <hongjiu.lu@intel.com> >>> >>> PR middle-end/55142 >>> * expr.c (expand_expr_real_2): Check Pmode instead of ptr_mode >>> for wrong address mode. >>> >>> gcc/testsuite/ >>> >>> 2012-11-03 H.J. Lu <hongjiu.lu@intel.com> >>> >>> PR middle-end/55142 >>> * gcc.target/i386/pr55142.c: New file. >>> >>> diff --git a/gcc/expr.c b/gcc/expr.c >>> index 0ad3b57..1600380 100644 >>> --- a/gcc/expr.c >>> +++ b/gcc/expr.c >>> @@ -8290,7 +8290,7 @@ expand_expr_real_2 (sepops ops, rtx target, enum machine_mode tmode, >>> And force_operand won't know whether to sign-extend or >>> zero-extend. */ >>> if ((modifier != EXPAND_SUM && modifier != EXPAND_INITIALIZER) >>> - || mode != ptr_mode) >>> + || mode != Pmode) >>> { >>> expand_operands (treeop0, treeop1, >>> subtarget, &op0, &op1, EXPAND_NORMAL); >>> @@ -8333,7 +8333,7 @@ expand_expr_real_2 (sepops ops, rtx target, enum machine_mode tmode, >>> And force_operand won't know whether to sign-extend or >>> zero-extend. */ >>> if ((modifier != EXPAND_SUM && modifier != EXPAND_INITIALIZER) >>> - || mode != ptr_mode) >>> + || mode != Pmode) >>> goto binop; >>> >>> expand_operands (treeop0, treeop1, >>> diff --git a/gcc/testsuite/gcc.target/i386/pr55142.c b/gcc/testsuite/gcc.target/i386/pr55142.c >>> new file mode 100644 >>> index 0000000..c8a9625 >>> --- /dev/null >>> +++ b/gcc/testsuite/gcc.target/i386/pr55142.c >>> @@ -0,0 +1,34 @@ >>> +/* { dg-do compile { target { ! { ia32 } } } } */ >>> +/* { dg-require-effective-target fpic } */ >>> +/* { dg-options "-O2 -mx32 -maddress-mode=long -fpic" } */ >>> + >>> +typedef int int32_t; >>> +typedef unsigned int uint32_t; >>> +typedef int32_t Elf32_Sword; >>> +typedef struct >>> +{ >>> + Elf32_Sword d_tag; >>> +} Elf32_Dyn; >>> +struct link_map >>> +{ >>> + Elf32_Dyn *l_ld; >>> + Elf32_Dyn *l_info[34]; >>> +}; >>> +extern struct link_map _dl_rtld_map __attribute__ ((visibility ("hidden"))); >>> +static void elf_get_dynamic_info (struct link_map *l) >>> +{ >>> + Elf32_Dyn *dyn = l->l_ld; >>> + Elf32_Dyn **info; >>> + info = l->l_info; >>> + while (dyn->d_tag != 0) >>> + { >>> + if ((uint32_t) (0x6ffffeff - dyn->d_tag) < 11) >>> + info[0x6ffffeff - dyn->d_tag + 12] = dyn; >>> + ++dyn; >>> + } >>> +} >>> +void >>> +foo (void) >>> +{ >>> + elf_get_dynamic_info (&_dl_rtld_map); >>> +} >> >> Any comments? I'd like to get it fixed for 4.8. > > I can't approve this anyway, but the existing ptr_mode sounds right for > EXPAND_INTIALIZER. I assume it would be for EXPAND_SUM too, although > I've always been a bit unsure what that means. Did you mean my patch was wrong to change ptr_mode to Pmode? Do you have any suggestions? This code has been this way when it was first checked into the current GCC tree back in Mar., 1992. >> This patch only impacts targets with Pmode != ptr_mode. Richard, can What are targets which will be affected by this change? Thanks.
On Mon, Nov 5, 2012 at 3:06 PM, Richard Sandiford <rdsandiford@googlemail.com> wrote: > "H.J. Lu" <hjl.tools@gmail.com> writes: >> On Sat, Nov 3, 2012 at 1:14 AM, H.J. Lu <hjl.tools@gmail.com> wrote: >>> Hi, >>> >>> The testcase shows -O -mx32 -maddress-mode=long -fPIC -S generates; >>> >>> x.i:22:37: internal compiler error: in plus_constant, at explow.c:88 >>> info[0x6ffffeff - dyn->d_tag + 12] = dyn; >>> >>> expand_expr_real_2 has >>> >>> /* No sense saving up arithmetic to be done >>> if it's all in the wrong mode to form part of an address. >>> And force_operand won't know whether to sign-extend or >>> zero-extend. */ >>> if ((modifier != EXPAND_SUM && modifier != EXPAND_INITIALIZER) >>> || mode != ptr_mode) >>> >>> With mode == SImode, Pmode == DImode and ptr_mode == SImode, we >>> generate wrong address. Instead of zero-extending address >>> 0xf7ffdd64, we sign-extend it to 0xfffffffff7ffdd64. This patch >>> checks Pmode instead of ptr_mode for address mode. It fixes the testcase >>> and generates a working x32 glibc. Is this patch correct? >>> >>> Thanks. >>> >>> >>> H.J. >>> --- >>> gcc/ >>> >>> 2012-11-03 H.J. Lu <hongjiu.lu@intel.com> >>> >>> PR middle-end/55142 >>> * expr.c (expand_expr_real_2): Check Pmode instead of ptr_mode >>> for wrong address mode. >>> >>> gcc/testsuite/ >>> >>> 2012-11-03 H.J. Lu <hongjiu.lu@intel.com> >>> >>> PR middle-end/55142 >>> * gcc.target/i386/pr55142.c: New file. >>> >>> diff --git a/gcc/expr.c b/gcc/expr.c >>> index 0ad3b57..1600380 100644 >>> --- a/gcc/expr.c >>> +++ b/gcc/expr.c >>> @@ -8290,7 +8290,7 @@ expand_expr_real_2 (sepops ops, rtx target, enum machine_mode tmode, >>> And force_operand won't know whether to sign-extend or >>> zero-extend. */ >>> if ((modifier != EXPAND_SUM && modifier != EXPAND_INITIALIZER) >>> - || mode != ptr_mode) >>> + || mode != Pmode) >>> { >>> expand_operands (treeop0, treeop1, >>> subtarget, &op0, &op1, EXPAND_NORMAL); >>> @@ -8333,7 +8333,7 @@ expand_expr_real_2 (sepops ops, rtx target, enum machine_mode tmode, >>> And force_operand won't know whether to sign-extend or >>> zero-extend. */ >>> if ((modifier != EXPAND_SUM && modifier != EXPAND_INITIALIZER) >>> - || mode != ptr_mode) >>> + || mode != Pmode) >>> goto binop; >>> >>> expand_operands (treeop0, treeop1, >>> diff --git a/gcc/testsuite/gcc.target/i386/pr55142.c b/gcc/testsuite/gcc.target/i386/pr55142.c >>> new file mode 100644 >>> index 0000000..c8a9625 >>> --- /dev/null >>> +++ b/gcc/testsuite/gcc.target/i386/pr55142.c >>> @@ -0,0 +1,34 @@ >>> +/* { dg-do compile { target { ! { ia32 } } } } */ >>> +/* { dg-require-effective-target fpic } */ >>> +/* { dg-options "-O2 -mx32 -maddress-mode=long -fpic" } */ >>> + >>> +typedef int int32_t; >>> +typedef unsigned int uint32_t; >>> +typedef int32_t Elf32_Sword; >>> +typedef struct >>> +{ >>> + Elf32_Sword d_tag; >>> +} Elf32_Dyn; >>> +struct link_map >>> +{ >>> + Elf32_Dyn *l_ld; >>> + Elf32_Dyn *l_info[34]; >>> +}; >>> +extern struct link_map _dl_rtld_map __attribute__ ((visibility ("hidden"))); >>> +static void elf_get_dynamic_info (struct link_map *l) >>> +{ >>> + Elf32_Dyn *dyn = l->l_ld; >>> + Elf32_Dyn **info; >>> + info = l->l_info; >>> + while (dyn->d_tag != 0) >>> + { >>> + if ((uint32_t) (0x6ffffeff - dyn->d_tag) < 11) >>> + info[0x6ffffeff - dyn->d_tag + 12] = dyn; >>> + ++dyn; >>> + } >>> +} >>> +void >>> +foo (void) >>> +{ >>> + elf_get_dynamic_info (&_dl_rtld_map); >>> +} >> >> Any comments? I'd like to get it fixed for 4.8. > > I can't approve this anyway, but the existing ptr_mode sounds right for > EXPAND_INTIALIZER. I assume it would be for EXPAND_SUM too, although > I've always been a bit unsure what that means. Did you mean my patch was wrong to change ptr_mode to Pmode? Do you have any suggestions? This code has been this way when it was first checked into the current GCC tree back in Mar., 1992. >> This patch only impacts targets with Pmode != ptr_mode. Richard, can What are targets which will be affected by this change? Thanks.
diff --git a/gcc/expr.c b/gcc/expr.c index 0ad3b57..1600380 100644 --- a/gcc/expr.c +++ b/gcc/expr.c @@ -8290,7 +8290,7 @@ expand_expr_real_2 (sepops ops, rtx target, enum machine_mode tmode, And force_operand won't know whether to sign-extend or zero-extend. */ if ((modifier != EXPAND_SUM && modifier != EXPAND_INITIALIZER) - || mode != ptr_mode) + || mode != Pmode) { expand_operands (treeop0, treeop1, subtarget, &op0, &op1, EXPAND_NORMAL); @@ -8333,7 +8333,7 @@ expand_expr_real_2 (sepops ops, rtx target, enum machine_mode tmode, And force_operand won't know whether to sign-extend or zero-extend. */ if ((modifier != EXPAND_SUM && modifier != EXPAND_INITIALIZER) - || mode != ptr_mode) + || mode != Pmode) goto binop; expand_operands (treeop0, treeop1, diff --git a/gcc/testsuite/gcc.target/i386/pr55142.c b/gcc/testsuite/gcc.target/i386/pr55142.c new file mode 100644 index 0000000..c8a9625 --- /dev/null +++ b/gcc/testsuite/gcc.target/i386/pr55142.c @@ -0,0 +1,34 @@ +/* { dg-do compile { target { ! { ia32 } } } } */ +/* { dg-require-effective-target fpic } */ +/* { dg-options "-O2 -mx32 -maddress-mode=long -fpic" } */ + +typedef int int32_t; +typedef unsigned int uint32_t; +typedef int32_t Elf32_Sword; +typedef struct +{ + Elf32_Sword d_tag; +} Elf32_Dyn; +struct link_map +{ + Elf32_Dyn *l_ld; + Elf32_Dyn *l_info[34]; +}; +extern struct link_map _dl_rtld_map __attribute__ ((visibility ("hidden"))); +static void elf_get_dynamic_info (struct link_map *l) +{ + Elf32_Dyn *dyn = l->l_ld; + Elf32_Dyn **info; + info = l->l_info; + while (dyn->d_tag != 0) + { + if ((uint32_t) (0x6ffffeff - dyn->d_tag) < 11) + info[0x6ffffeff - dyn->d_tag + 12] = dyn; + ++dyn; + } +} +void +foo (void) +{ + elf_get_dynamic_info (&_dl_rtld_map); +}
Hi, The testcase shows -O -mx32 -maddress-mode=long -fPIC -S generates; x.i:22:37: internal compiler error: in plus_constant, at explow.c:88 info[0x6ffffeff - dyn->d_tag + 12] = dyn; expand_expr_real_2 has /* No sense saving up arithmetic to be done if it's all in the wrong mode to form part of an address. And force_operand won't know whether to sign-extend or zero-extend. */ if ((modifier != EXPAND_SUM && modifier != EXPAND_INITIALIZER) || mode != ptr_mode) With mode == SImode, Pmode == DImode and ptr_mode == SImode, we generate wrong address. Instead of zero-extending address 0xf7ffdd64, we sign-extend it to 0xfffffffff7ffdd64. This patch checks Pmode instead of ptr_mode for address mode. It fixes the testcase and generates a working x32 glibc. Is this patch correct? Thanks. H.J. --- gcc/ 2012-11-03 H.J. Lu <hongjiu.lu@intel.com> PR middle-end/55142 * expr.c (expand_expr_real_2): Check Pmode instead of ptr_mode for wrong address mode. gcc/testsuite/ 2012-11-03 H.J. Lu <hongjiu.lu@intel.com> PR middle-end/55142 * gcc.target/i386/pr55142.c: New file.