PING: PING: patch: ICE in legitimize_pic_address for x86_64 mingw target

PING: PING: patch: ICE in legitimize_pic_address for x86_64 mingw target

[Kai Tietz]

[-- Attachment #1: Type: text/plain, Size: 1367 bytes --]

Hi,

there is an ICE for __declspec(dllimport) imported variables. The checking 
of pic seems to be to early in legitimize_address. The check for 
TARGET_DLLIMPORT_DECL_ATTRIBUTES should be done infront AFAICS. I 
bootstraped it for x86_64-pc-mingw32 and i686-pc-mignew32 and it seems to 
work fine for me.
I tested this patch for i686-pc-cygwin and x86_64-pc-mingw32 and I saw no 
regressions.

ChangeLog:

2007-03-30  Kai Tietz  <kai.tietz@onevision.com>

        * i386.c: (legitimize_address): Move dllimported variable check
        infront of legitimizing pic address of CONST symbols.

Regards,
 i.A. Kai Tietz


|  (\_/)  This is Bunny. Copy and paste Bunny
| (='.'=) into your signature to help him gain
| (")_(") world domination.

------------------------------------------------------------------------------------------
  OneVision Software Entwicklungs GmbH & Co. KG
  Dr.-Leo-Ritter-Straße 9 - 93049 Regensburg
  Tel: +49.(0)941.78004.0 - Fax: +49.(0)941.78004.489 - www.OneVision.com
  Commerzbank Regensburg - BLZ 750 400 62 - Konto 6011050
  Handelsregister: HRA 6744, Amtsgericht Regensburg
  Komplementärin: OneVision Software Entwicklungs Verwaltungs GmbH
  Dr.-Leo-Ritter-Straße 9 – 93049 Regensburg
  Handelsregister: HRB 8932, Amtsgericht Regensburg - Geschäftsführer: 
Ulrike Döhler, Manuela Kluger

[-- Attachment #2: i386_got.txt --]
[-- Type: text/plain, Size: 812 bytes --]

Index: gcc/gcc/config/i386/i386.c
===================================================================
--- gcc.orig/gcc/config/i386/i386.c
+++ gcc/gcc/config/i386/i386.c
@@ -7673,9 +7673,6 @@ legitimize_address (rtx x, rtx oldx ATTR
       return gen_rtx_PLUS (Pmode, t, XEXP (XEXP (x, 0), 1));
     }
 
-  if (flag_pic && SYMBOLIC_CONST (x))
-    return legitimize_pic_address (x, 0);
-
   if (TARGET_DLLIMPORT_DECL_ATTRIBUTES)
     {
       if (GET_CODE (x) == SYMBOL_REF && SYMBOL_REF_DLLIMPORT_P (x))
@@ -7690,6 +7687,9 @@ legitimize_address (rtx x, rtx oldx ATTR
 	}
     }
 
+  if (flag_pic && SYMBOLIC_CONST (x))
+    return legitimize_pic_address (x, 0);
+
   /* Canonicalize shifts by 0, 1, 2, 3 into multiply */
   if (GET_CODE (x) == ASHIFT
       && CONST_INT_P (XEXP (x, 1))
=

Re: PING: PING: patch: ICE in legitimize_pic_address for x86_64 mingw target

[Jan Hubicka]

> Hi,
> 
> there is an ICE for __declspec(dllimport) imported variables. The checking 
> of pic seems to be to early in legitimize_address. The check for 
> TARGET_DLLIMPORT_DECL_ATTRIBUTES should be done infront AFAICS. I 
> bootstraped it for x86_64-pc-mingw32 and i686-pc-mignew32 and it seems to 
> work fine for me.
> I tested this patch for i686-pc-cygwin and x86_64-pc-mingw32 and I saw no 
> regressions.

Hi,
the patch looks fine, however I think you need to check range of
CONST_INT operand:
      if (GET_CODE (x) == CONST
          && GET_CODE (XEXP (x, 0)) == PLUS
	  && GET_CODE (XEXP (XEXP (x, 0), 0)) == SYMBOL_REF
	  && SYMBOL_REF_DLLIMPORT_P (XEXP (XEXP (x, 0), 0)))

so the offset won't end up exceeding 32bit encoding. Does Windows ABI
guarantee some constant value?  (GCC can generate constant offset that
points off the symbol's object memory area itself when simplifying more
complex memory references)

SYSV ABI reserve 16MB for this that is checked at few places:
          if (!CONST_INT_P (op1) || INTVAL (op1) >= 16*1024*1024 || INTVAL (op1) < -16*1024*1024)
				       
Honza
> 
> ChangeLog:
> 
> 2007-03-30  Kai Tietz  <kai.tietz@onevision.com>
> 
>         * i386.c: (legitimize_address): Move dllimported variable check
>         infront of legitimizing pic address of CONST symbols.
> 
> Regards,
>  i.A. Kai Tietz
> 
> 
> |  (\_/)  This is Bunny. Copy and paste Bunny
> | (='.'=) into your signature to help him gain
> | (")_(") world domination.
> 
> ------------------------------------------------------------------------------------------
>   OneVision Software Entwicklungs GmbH & Co. KG
>   Dr.-Leo-Ritter-StraĂŸe 9 - 93049 Regensburg
>   Tel: +49.(0)941.78004.0 - Fax: +49.(0)941.78004.489 - www.OneVision.com
>   Commerzbank Regensburg - BLZ 750 400 62 - Konto 6011050
>   Handelsregister: HRA 6744, Amtsgericht Regensburg
>   Komplementärin: OneVision Software Entwicklungs Verwaltungs GmbH
>   Dr.-Leo-Ritter-StraĂŸe 9 ??? 93049 Regensburg
>   Handelsregister: HRB 8932, Amtsgericht Regensburg - Geschäftsfßhrer: 
> Ulrike DĂśhler, Manuela Kluger

> Index: gcc/gcc/config/i386/i386.c
> ===================================================================
> --- gcc.orig/gcc/config/i386/i386.c
> +++ gcc/gcc/config/i386/i386.c
> @@ -7673,9 +7673,6 @@ legitimize_address (rtx x, rtx oldx ATTR
>        return gen_rtx_PLUS (Pmode, t, XEXP (XEXP (x, 0), 1));
>      }
>  
> -  if (flag_pic && SYMBOLIC_CONST (x))
> -    return legitimize_pic_address (x, 0);
> -
>    if (TARGET_DLLIMPORT_DECL_ATTRIBUTES)
>      {
>        if (GET_CODE (x) == SYMBOL_REF && SYMBOL_REF_DLLIMPORT_P (x))
> @@ -7690,6 +7687,9 @@ legitimize_address (rtx x, rtx oldx ATTR
>  	}
>      }
>  
> +  if (flag_pic && SYMBOLIC_CONST (x))
> +    return legitimize_pic_address (x, 0);
> +
>    /* Canonicalize shifts by 0, 1, 2, 3 into multiply */
>    if (GET_CODE (x) == ASHIFT
>        && CONST_INT_P (XEXP (x, 1))
> =

Re: PING: PING: patch: ICE in legitimize_pic_address for x86_64 mingw target

[Kai Tietz]

Hi Jan,

Jan Hubicka wrote on 31.07.2007 23:36:47:

> > Hi,
> > 
> > there is an ICE for __declspec(dllimport) imported variables. The 
checking 
> > of pic seems to be to early in legitimize_address. The check for 
> > TARGET_DLLIMPORT_DECL_ATTRIBUTES should be done infront AFAICS. I 
> > bootstraped it for x86_64-pc-mingw32 and i686-pc-mignew32 and it seems 
to 
> > work fine for me.
> > I tested this patch for i686-pc-cygwin and x86_64-pc-mingw32 and I saw 
no 
> > regressions.
> 
> Hi,
> the patch looks fine, however I think you need to check range of
> CONST_INT operand:
>       if (GET_CODE (x) == CONST
>           && GET_CODE (XEXP (x, 0)) == PLUS
>      && GET_CODE (XEXP (XEXP (x, 0), 0)) == SYMBOL_REF
>      && SYMBOL_REF_DLLIMPORT_P (XEXP (XEXP (x, 0), 0)))
> 
> so the offset won't end up exceeding 32bit encoding. Does Windows ABI
> guarantee some constant value?  (GCC can generate constant offset that
> points off the symbol's object memory area itself when simplifying more
> complex memory references)
AFAIK there are no quaranteed constant values beside the gs/fs-segment 
area in the 64-bit ABI of MS.
So I think the case described by you won't occure, but may I missed 
something.
The MS ABI defines base relocations 64-bit wide. There is no 32-bit base 
relocation for this target. They have to be PC-relative. But this target 
defines PC-relative relocations with 8, 32, and 64 bit width. So I don't 
expect problems there.

Cheers,
 i.A. Kai Tietz

|  (\_/)  This is Bunny. Copy and paste Bunny
| (='.'=) into your signature to help him gain
| (")_(") world domination.

------------------------------------------------------------------------------------------
  OneVision Software Entwicklungs GmbH & Co. KG
  Dr.-Leo-Ritter-Straße 9 - 93049 Regensburg
  Tel: +49.(0)941.78004.0 - Fax: +49.(0)941.78004.489 - www.OneVision.com
  Commerzbank Regensburg - BLZ 750 400 62 - Konto 6011050
  Handelsregister: HRA 6744, Amtsgericht Regensburg
  Komplementärin: OneVision Software Entwicklungs Verwaltungs GmbH
  Dr.-Leo-Ritter-Straße 9 – 93049 Regensburg
  Handelsregister: HRB 8932, Amtsgericht Regensburg - Geschäftsführer: 
Ulrike Döhler, Manuela Kluger

Re: PING: PING: patch: ICE in legitimize_pic_address for x86_64 mingw target

[Jan Hubicka]

> AFAIK there are no quaranteed constant values beside the gs/fs-segment 
> area in the 64-bit ABI of MS.
> So I think the case described by you won't occure, but may I missed 
> something.
> The MS ABI defines base relocations 64-bit wide. There is no 32-bit base 
> relocation for this target. They have to be PC-relative. But this target 
> defines PC-relative relocations with 8, 32, and 64 bit width. So I don't 
> expect problems there.

There is no 64bit pc relative addressing mode on x86-64. The code in
question is matching base+offset addressing mode that is limited to
32bit signed values. Thinking about it, the generic address legitimizing
code is going to work this out, so you probably don't need to care about
it. 

The patch is OK
Honza
> AA
> Cheers,
>  i.A. Kai Tietz
> 
> |  (\_/)  This is Bunny. Copy and paste Bunny
> | (='.'=) into your signature to help him gain
> | (")_(") world domination.
> 
> ------------------------------------------------------------------------------------------
>   OneVision Software Entwicklungs GmbH & Co. KG
>   Dr.-Leo-Ritter-StraĂŸe 9 - 93049 Regensburg
>   Tel: +49.(0)941.78004.0 - Fax: +49.(0)941.78004.489 - www.OneVision.com
>   Commerzbank Regensburg - BLZ 750 400 62 - Konto 6011050
>   Handelsregister: HRA 6744, Amtsgericht Regensburg
>   Komplementärin: OneVision Software Entwicklungs Verwaltungs GmbH
>   Dr.-Leo-Ritter-StraĂŸe 9 ??? 93049 Regensburg
>   Handelsregister: HRB 8932, Amtsgericht Regensburg - Geschäftsfßhrer: 
> Ulrike DĂśhler, Manuela Kluger
>