RFA minor DF cleanup

RFA minor DF cleanup

[Jeff Law]

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

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1


As I've noted in prior messages; I'm looking to improve our path
isolation to improve code generation and reduce false positives from
warnings.

The patch that's been in my queue for some time now (and I suspect it's
the final patch to our current implementation of jump threading) is
capable of isolating more paths, but is certainly not capable of fully
isolating every optimizable path through the CFG and eliminating all
unexecutable paths through the CFG (neither of which is actually
desirable due to potential code bloat issues).

As a result of this better, but not full isolation, we can end up
exposing a constant propagation in a unexecutable path through the CFG
that isn't detected as unexecutable.  As a result of exposing the
constant propagation we can trigger a bogus warning from -Warray-bounds.

The problem is we might have something like this:

>   # BLOCK 11 freq:4946
>   # PRED: 9 [50.0%]  (false,exec) 10 [100.0%]  (fallthru,exec) 8 [28.0%]
>  (false,exec)
> Invalid sum of incoming frequencies 2819, should be 4946
>   # D.39048_1 = PHI <3(9), D.39048_19(10), 4294967295(8)>
>   # VUSE <.MEM_38(D)>
>   D.39016_24 = default_target_hard_regs.x_fixed_regs[D.39048_1];
> 


- -Warray-bounds won't warn for this as it only triggers when we propagate
a constant for an array index and the constant is out of bounds of the
array.    In this case D.39048_1 is not a constant and thus
- -Warray-bounds does not issues a warning.


The patch I've got queued up will isolate the path 8->9 (to optimize
elsewhere).  This results in a new block which looks like:

temp = PHI (4294967295);
D.39016_xx = default_target_hard_regs.x_fixed_regs[temp];

We then propagate the constant into the use of temp triggering the
- -Warray-bounds warning.

This is caused by this code fragment:

>       /* Any constant, or pseudo with constant equivalences, may
>          require reloading from memory using the pic register.  */
>       if ((unsigned) PIC_OFFSET_TABLE_REGNUM != INVALID_REGNUM
>           && fixed_regs[PIC_OFFSET_TABLE_REGNUM])
>         bitmap_set_bit (regular_block_artificial_uses, PIC_OFFSET_TABLE_REGNUM);

combined with this code from the x86 backend:

> #define PIC_OFFSET_TABLE_REGNUM                         \
>   ((TARGET_64BIT && ix86_cmodel == CM_SMALL_PIC)        \
>    || !flag_pic ? INVALID_REGNUM                        \
>    : reload_completed ? REGNO (pic_offset_table_rtx)    \
>    : REAL_PIC_OFFSET_TABLE_REGNUM)


While the new code can significantly improve path isolation, it's unable
to fully isolate the paths in this code, leading to the partial
isolation and exposing the constant propagation in the dead path which
triggers -Warray-bounds warning.

I'm hoping the ideas I'm working on for revamping how we handle path
isolation may fix this, but it's hard to be sure right now.  In the mean
time, this patch fixes the instances where the next improvements to jump
threading expose the bogus -Warray-bounds warning.

Bootstrapped and regression tested on x86_64-unknown-linux-gnu.  OK for
mainline?

Thanks,
Jeff





-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/

iQEcBAEBAgAGBQJN94aoAAoJEBRtltQi2kC7DwkIAI2zu87P0mqwf+NzI3BAPQpU
GQl9d2Lw4z7diUfn7k+q2OqZMaoof9L0CqvhqC07Pz+UGzpke28o2WoS2Jrwxbj9
eQzC/H5DcAXmazvkwpe0BphvtqD+2Puz3pilQG1Nyopi1xJB5aKhC55VLntQuAvy
+yaw/ozJ/d0Gt9myR/NXLe0NPfRycDeuC6U+iYRolJ7I/PxP/gZZ5dW68xakstLp
oaQOakKmTres7CMWqG6ZV+5KJyQU92rnkp4ympKZGkciK1yI7Bl8fA87SqY/QkzN
eDoGP37hQnJZkh39QLQjOZCfU5ywVAP81BnYsjaeSAOEd/SdQA63nIzVhGXoDEA=
=K4dB
-----END PGP SIGNATURE-----

[-- Attachment #2: patch2 --]
[-- Type: text/plain, Size: 5635 bytes --]

	* df-problems.c (df_lr_local_compute): Manually CSE
	PIC_OFFSET_TABLE_REGNUM.
	* df-scan.c (df_get_regular_block_artificial_uses): Likewise.
	(df_get_entry_block_def_set, df_get_exit_block_use_set): Likewise.

Index: df-problems.c
===================================================================
*** df-problems.c	(revision 174927)
--- df-problems.c	(working copy)
*************** df_lr_local_compute (bitmap all_blocks A
*** 906,911 ****
--- 906,912 ----
       blocks within infinite loops.  */
    if (!reload_completed)
      {
+       unsigned int pic_offset_table_regnum = PIC_OFFSET_TABLE_REGNUM;
        /* Any reference to any pseudo before reload is a potential
  	 reference of the frame pointer.  */
        bitmap_set_bit (&df->hardware_regs_used, FRAME_POINTER_REGNUM);
*************** df_lr_local_compute (bitmap all_blocks A
*** 919,927 ****
  
        /* Any constant, or pseudo with constant equivalences, may
  	 require reloading from memory using the pic register.  */
!       if ((unsigned) PIC_OFFSET_TABLE_REGNUM != INVALID_REGNUM
! 	  && fixed_regs[PIC_OFFSET_TABLE_REGNUM])
! 	bitmap_set_bit (&df->hardware_regs_used, PIC_OFFSET_TABLE_REGNUM);
      }
  
    EXECUTE_IF_SET_IN_BITMAP (df_lr->out_of_date_transfer_functions, 0, bb_index, bi)
--- 920,928 ----
  
        /* Any constant, or pseudo with constant equivalences, may
  	 require reloading from memory using the pic register.  */
!       if (pic_offset_table_regnum != INVALID_REGNUM
! 	  && fixed_regs[pic_offset_table_regnum])
! 	bitmap_set_bit (&df->hardware_regs_used, pic_offset_table_regnum);
      }
  
    EXECUTE_IF_SET_IN_BITMAP (df_lr->out_of_date_transfer_functions, 0, bb_index, bi)
Index: df-scan.c
===================================================================
*** df-scan.c	(revision 174927)
--- df-scan.c	(working copy)
*************** df_get_regular_block_artificial_uses (bi
*** 3625,3630 ****
--- 3625,3632 ----
         live everywhere -- which might not already be the case for
         blocks within infinite loops.  */
      {
+       unsigned int picreg = PIC_OFFSET_TABLE_REGNUM;
+ 
        /* Any reference to any pseudo before reload is a potential
  	 reference of the frame pointer.  */
        bitmap_set_bit (regular_block_artificial_uses, FRAME_POINTER_REGNUM);
*************** df_get_regular_block_artificial_uses (bi
*** 3642,3650 ****
  
        /* Any constant, or pseudo with constant equivalences, may
  	 require reloading from memory using the pic register.  */
!       if ((unsigned) PIC_OFFSET_TABLE_REGNUM != INVALID_REGNUM
! 	  && fixed_regs[PIC_OFFSET_TABLE_REGNUM])
! 	bitmap_set_bit (regular_block_artificial_uses, PIC_OFFSET_TABLE_REGNUM);
      }
    /* The all-important stack pointer must always be live.  */
    bitmap_set_bit (regular_block_artificial_uses, STACK_POINTER_REGNUM);
--- 3644,3652 ----
  
        /* Any constant, or pseudo with constant equivalences, may
  	 require reloading from memory using the pic register.  */
!       if (picreg != INVALID_REGNUM
! 	  && fixed_regs[picreg])
! 	bitmap_set_bit (regular_block_artificial_uses, picreg);
      }
    /* The all-important stack pointer must always be live.  */
    bitmap_set_bit (regular_block_artificial_uses, STACK_POINTER_REGNUM);
*************** df_get_entry_block_def_set (bitmap entry
*** 3779,3784 ****
--- 3781,3790 ----
    /* These registers are live everywhere.  */
    if (!reload_completed)
      {
+ #ifdef PIC_OFFSET_TABLE_REGNUM
+       unsigned int picreg = PIC_OFFSET_TABLE_REGNUM;
+ #endif
+ 
  #if FRAME_POINTER_REGNUM != ARG_POINTER_REGNUM
        /* Pseudos with argument area equivalences may require
  	 reloading via the argument pointer.  */
*************** df_get_entry_block_def_set (bitmap entry
*** 3789,3797 ****
  #ifdef PIC_OFFSET_TABLE_REGNUM
        /* Any constant, or pseudo with constant equivalences, may
  	 require reloading from memory using the pic register.  */
!       if ((unsigned) PIC_OFFSET_TABLE_REGNUM != INVALID_REGNUM
! 	  && fixed_regs[PIC_OFFSET_TABLE_REGNUM])
! 	bitmap_set_bit (entry_block_defs, PIC_OFFSET_TABLE_REGNUM);
  #endif
      }
  
--- 3795,3803 ----
  #ifdef PIC_OFFSET_TABLE_REGNUM
        /* Any constant, or pseudo with constant equivalences, may
  	 require reloading from memory using the pic register.  */
!       if (picreg != INVALID_REGNUM
! 	  && fixed_regs[picreg])
! 	bitmap_set_bit (entry_block_defs, picreg);
  #endif
      }
  
*************** static void
*** 3889,3894 ****
--- 3895,3901 ----
  df_get_exit_block_use_set (bitmap exit_block_uses)
  {
    unsigned int i;
+   unsigned int picreg = PIC_OFFSET_TABLE_REGNUM;
  
    bitmap_clear (exit_block_uses);
  
*************** df_get_exit_block_use_set (bitmap exit_b
*** 3913,3921 ****
       Assume the pic register is not in use, or will be handled by
       other means, if it is not fixed.  */
    if (!PIC_OFFSET_TABLE_REG_CALL_CLOBBERED
!       && (unsigned) PIC_OFFSET_TABLE_REGNUM != INVALID_REGNUM
!       && fixed_regs[PIC_OFFSET_TABLE_REGNUM])
!     bitmap_set_bit (exit_block_uses, PIC_OFFSET_TABLE_REGNUM);
  
    /* Mark all global registers, and all registers used by the
       epilogue as being live at the end of the function since they
--- 3920,3928 ----
       Assume the pic register is not in use, or will be handled by
       other means, if it is not fixed.  */
    if (!PIC_OFFSET_TABLE_REG_CALL_CLOBBERED
!       && picreg != INVALID_REGNUM
!       && fixed_regs[picreg])
!     bitmap_set_bit (exit_block_uses, picreg);
  
    /* Mark all global registers, and all registers used by the
       epilogue as being live at the end of the function since they

Re: RFA minor DF cleanup

[Richard Guenther]

On Tue, Jun 14, 2011 at 6:04 PM, Jeff Law <law@redhat.com> wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
>
> As I've noted in prior messages; I'm looking to improve our path
> isolation to improve code generation and reduce false positives from
> warnings.
>
> The patch that's been in my queue for some time now (and I suspect it's
> the final patch to our current implementation of jump threading) is
> capable of isolating more paths, but is certainly not capable of fully
> isolating every optimizable path through the CFG and eliminating all
> unexecutable paths through the CFG (neither of which is actually
> desirable due to potential code bloat issues).
>
> As a result of this better, but not full isolation, we can end up
> exposing a constant propagation in a unexecutable path through the CFG
> that isn't detected as unexecutable.  As a result of exposing the
> constant propagation we can trigger a bogus warning from -Warray-bounds.
>
> The problem is we might have something like this:
>
>>   # BLOCK 11 freq:4946
>>   # PRED: 9 [50.0%]  (false,exec) 10 [100.0%]  (fallthru,exec) 8 [28.0%]
>>  (false,exec)
>> Invalid sum of incoming frequencies 2819, should be 4946
>>   # D.39048_1 = PHI <3(9), D.39048_19(10), 4294967295(8)>
>>   # VUSE <.MEM_38(D)>
>>   D.39016_24 = default_target_hard_regs.x_fixed_regs[D.39048_1];
>>
>
>
> - -Warray-bounds won't warn for this as it only triggers when we propagate
> a constant for an array index and the constant is out of bounds of the
> array.    In this case D.39048_1 is not a constant and thus
> - -Warray-bounds does not issues a warning.
>
>
> The patch I've got queued up will isolate the path 8->9 (to optimize
> elsewhere).  This results in a new block which looks like:
>
> temp = PHI (4294967295);
> D.39016_xx = default_target_hard_regs.x_fixed_regs[temp];
>
> We then propagate the constant into the use of temp triggering the
> - -Warray-bounds warning.
>
> This is caused by this code fragment:
>
>>       /* Any constant, or pseudo with constant equivalences, may
>>          require reloading from memory using the pic register.  */
>>       if ((unsigned) PIC_OFFSET_TABLE_REGNUM != INVALID_REGNUM
>>           && fixed_regs[PIC_OFFSET_TABLE_REGNUM])
>>         bitmap_set_bit (regular_block_artificial_uses, PIC_OFFSET_TABLE_REGNUM);
>
> combined with this code from the x86 backend:
>
>> #define PIC_OFFSET_TABLE_REGNUM                         \
>>   ((TARGET_64BIT && ix86_cmodel == CM_SMALL_PIC)        \
>>    || !flag_pic ? INVALID_REGNUM                        \
>>    : reload_completed ? REGNO (pic_offset_table_rtx)    \
>>    : REAL_PIC_OFFSET_TABLE_REGNUM)
>
>
> While the new code can significantly improve path isolation, it's unable
> to fully isolate the paths in this code, leading to the partial
> isolation and exposing the constant propagation in the dead path which
> triggers -Warray-bounds warning.
>
> I'm hoping the ideas I'm working on for revamping how we handle path
> isolation may fix this, but it's hard to be sure right now.  In the mean
> time, this patch fixes the instances where the next improvements to jump
> threading expose the bogus -Warray-bounds warning.
>
> Bootstrapped and regression tested on x86_64-unknown-linux-gnu.  OK for
> mainline?

Ok.

Thanks,
Richard.

> Thanks,
> Jeff
>
>
>
>
>
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.4.11 (GNU/Linux)
> Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/
>
> iQEcBAEBAgAGBQJN94aoAAoJEBRtltQi2kC7DwkIAI2zu87P0mqwf+NzI3BAPQpU
> GQl9d2Lw4z7diUfn7k+q2OqZMaoof9L0CqvhqC07Pz+UGzpke28o2WoS2Jrwxbj9
> eQzC/H5DcAXmazvkwpe0BphvtqD+2Puz3pilQG1Nyopi1xJB5aKhC55VLntQuAvy
> +yaw/ozJ/d0Gt9myR/NXLe0NPfRycDeuC6U+iYRolJ7I/PxP/gZZ5dW68xakstLp
> oaQOakKmTres7CMWqG6ZV+5KJyQU92rnkp4ympKZGkciK1yI7Bl8fA87SqY/QkzN
> eDoGP37hQnJZkh39QLQjOZCfU5ywVAP81BnYsjaeSAOEd/SdQA63nIzVhGXoDEA=
> =K4dB
> -----END PGP SIGNATURE-----
>