Re: RFC: PR middle-end/55142: Check Pmode instead of ptr_mode for address mode

[Richard Sandiford <rdsandiford at googlemail dot com>]

"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