Re: RFA minor DF cleanup

2011-06-15 Thread Richard Guenther
On Tue, Jun 14, 2011 at 6:04 PM, Jeff Law  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-
>


RFA minor DF cleanup

2011-06-14 Thread Jeff Law
-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-
* 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,