Re: [PATCH] Fix PR89497

[Richard Biener <rguenther at suse dot de>]

On March 4, 2019 5:23:42 PM GMT+01:00, Christophe Lyon <christophe.lyon@linaro.org> wrote:
>On Fri, 1 Mar 2019 at 10:20, Richard Biener <rguenther@suse.de> wrote:
>>
>> On Wed, 27 Feb 2019, Richard Biener wrote:
>>
>> >
>> > CFG cleanup is now set up to perform trivial unreachable code
>> > elimination before doing anything that would require up-to-date
>> > SSA form.  Unfortunately a pending SSA update still will cause
>> > breakage to stmt folding triggered for example by basic-block
>> > merging.
>> >
>> > Fortunately it's now easy to properly "interleave" CFG cleanup
>> > and SSA update.
>> >
>> > Done as follows, bootstrap & regtest running on
>x86_64-unknown-linux-gnu.
>>
>> Testing went OK, two testcases need adjustments though.
>>
>> FAIL: gcc.dg/tree-ssa/reassoc-43.c scan-tree-dump-not reassoc2 "0 !=
>0"
>>
>> here we now end up with if (_20 != 0) matching.
>>
>> FAIL: g++.dg/tree-prof/devirt.C scan-tree-dump-times dom3 "folding
>virtual
>> function call to virtual unsigned int mozPersonalDictionary::AddRef"
>1
>> FAIL: g++.dg/tree-prof/devirt.C scan-tree-dump-times dom3 "folding
>virtual
>> function call to virtual unsigned int mozPersonalDictionary::_ZThn16"
>1
>>
>> here both foldings now alrady happen one pass earlier (during tracer
>> triggered CFG cleanup).  Previously the folding didn't happen because
>> the SSA names were marked for SSA update.
>>
>> Committed as follows.
>>
>
>Hi Richard,
>
>I've noticed a regression after you committed this patch:
>FAIL: gcc.dg/uninit-pred-8_b.c bogus warning (test for bogus messages,
>line 20)
>FAIL: gcc.dg/uninit-pred-8_b.c bogus warning (test for bogus messages,
>line 39)
>FAIL: gcc.dg/uninit-pred-8_b.c warning (test for warnings, line 42)
>
>It's unusual because I see that on arm-none-linux-gnueabihf
>--with-cpu cortex-a5
>--with-fpu vfpv3-d16-fp16
>
>but the same test still passes on the same target
>--with-cpu cortex-a9
>--with-fpu neon-fp16
>
>Any idea?

See PR89551. 

Richard. 

>Thanks,
>
>Christophe
>
>> Richard.
>>
>> 2019-03-01  Richard Biener  <rguenther@suse.de>
>>
>>         PR middle-end/89497
>>         * tree-cfgcleanup.h (cleanup_tree_cfg): Add SSA update flags
>>         argument, defaulted to zero.
>>         * passes.c (execute_function_todo): Pass down SSA update
>flags
>>         to cleanup_tree_cfg.
>>         * tree-cfgcleanup.c: Include tree-into-ssa.h and
>tree-cfgcleanup.h.
>>         (cleanup_tree_cfg_noloop): After cleanup_control_flow_pre
>update SSA
>>         form if requested.
>>         (cleanup_tree_cfg): Get and pass down SSA update flags.
>>
>>         * gcc.dg/tree-ssa/reassoc-43.c: Avoid false match in regex.
>>         * g++.dg/tree-prof/devirt.C: Scan tracer dump for foldings
>>         that happen now earlier.
>>
>> Index: gcc/tree-cfgcleanup.h
>> ===================================================================
>> --- gcc/tree-cfgcleanup.h       (revision 269251)
>> +++ gcc/tree-cfgcleanup.h       (working copy)
>> @@ -22,7 +22,7 @@ along with GCC; see the file COPYING3.
>>
>>  /* In tree-cfgcleanup.c  */
>>  extern bitmap cfgcleanup_altered_bbs;
>> -extern bool cleanup_tree_cfg (void);
>> +extern bool cleanup_tree_cfg (unsigned = 0);
>>  extern bool fixup_noreturn_call (gimple *stmt);
>>  extern bool delete_unreachable_blocks_update_callgraph (cgraph_node
>*dst_node,
>>                                                         bool
>update_clones);
>> Index: gcc/passes.c
>> ===================================================================
>> --- gcc/passes.c        (revision 269251)
>> +++ gcc/passes.c        (working copy)
>> @@ -1927,7 +1927,7 @@ execute_function_todo (function *fn, voi
>>    /* Always cleanup the CFG before trying to update SSA.  */
>>    if (flags & TODO_cleanup_cfg)
>>      {
>> -      cleanup_tree_cfg ();
>> +      cleanup_tree_cfg (flags & TODO_update_ssa_any);
>>
>>        /* When cleanup_tree_cfg merges consecutive blocks, it may
>>          perform some simplistic propagation when removing single
>> Index: gcc/tree-cfgcleanup.c
>> ===================================================================
>> --- gcc/tree-cfgcleanup.c       (revision 269251)
>> +++ gcc/tree-cfgcleanup.c       (working copy)
>> @@ -44,6 +44,9 @@ along with GCC; see the file COPYING3.
>>  #include "gimple-fold.h"
>>  #include "tree-ssa-loop-niter.h"
>>  #include "cgraph.h"
>> +#include "tree-into-ssa.h"
>> +#include "tree-cfgcleanup.h"
>> +
>>
>>  /* The set of blocks in that at least one of the following changes
>happened:
>>     -- the statement at the end of the block was changed
>> @@ -943,7 +946,7 @@ mfb_keep_latches (edge e)
>>     Return true if the flowgraph was modified, false otherwise.  */
>>
>>  static bool
>> -cleanup_tree_cfg_noloop (void)
>> +cleanup_tree_cfg_noloop (unsigned ssa_update_flags)
>>  {
>>    timevar_push (TV_TREE_CLEANUP_CFG);
>>
>> @@ -1023,6 +1026,8 @@ cleanup_tree_cfg_noloop (void)
>>
>>    /* After doing the above SSA form should be valid (or an update
>SSA
>>       should be required).  */
>> +  if (ssa_update_flags)
>> +    update_ssa (ssa_update_flags);
>>
>>    /* Compute dominator info which we need for the iterative process
>below.  */
>>    if (!dom_info_available_p (CDI_DOMINATORS))
>> @@ -1125,9 +1130,9 @@ repair_loop_structures (void)
>>  /* Cleanup cfg and repair loop structures.  */
>>
>>  bool
>> -cleanup_tree_cfg (void)
>> +cleanup_tree_cfg (unsigned ssa_update_flags)
>>  {
>> -  bool changed = cleanup_tree_cfg_noloop ();
>> +  bool changed = cleanup_tree_cfg_noloop (ssa_update_flags);
>>
>>    if (current_loops != NULL
>>        && loops_state_satisfies_p (LOOPS_NEED_FIXUP))
>> Index: gcc/testsuite/g++.dg/tree-prof/devirt.C
>> ===================================================================
>> --- gcc/testsuite/g++.dg/tree-prof/devirt.C     (revision 269301)
>> +++ gcc/testsuite/g++.dg/tree-prof/devirt.C     (working copy)
>> @@ -1,5 +1,5 @@
>>  /* PR ipa/88561 */
>> -/* { dg-options "-O3 -fdump-tree-dom3-details" } */
>> +/* { dg-options "-O3 -fdump-tree-tracer-details
>-fdump-tree-dom3-details" } */
>>
>>  struct nsISupports
>>  {
>> @@ -121,6 +121,6 @@ main ()
>>      __builtin_abort ();
>>  }
>>
>> -/* { dg-final-use-not-autofdo { scan-tree-dump-times "folding
>virtual function call to virtual unsigned int
>mozPersonalDictionary::_ZThn16" 1 "dom3" { target { lp64 || llp64 } } }
>} */
>> +/* { dg-final-use-not-autofdo { scan-tree-dump-times "folding
>virtual function call to virtual unsigned int
>mozPersonalDictionary::_ZThn16" 1 "tracer" { target { lp64 || llp64 } }
>} } */
>>  /* { dg-final-use-not-autofdo { scan-tree-dump-times "folding
>virtual function call to virtual unsigned int
>mozPersonalDictionary::_ZThn8" 1 "dom3" { target ilp32 } } } */
>> -/* { dg-final-use-not-autofdo { scan-tree-dump-times "folding
>virtual function call to virtual unsigned int
>mozPersonalDictionary::AddRef" 1 "dom3" } } */
>> +/* { dg-final-use-not-autofdo { scan-tree-dump-times "folding
>virtual function call to virtual unsigned int
>mozPersonalDictionary::AddRef" 1 "tracer" } } */
>> Index: gcc/testsuite/gcc.dg/tree-ssa/reassoc-43.c
>> ===================================================================
>> --- gcc/testsuite/gcc.dg/tree-ssa/reassoc-43.c  (revision 269301)
>> +++ gcc/testsuite/gcc.dg/tree-ssa/reassoc-43.c  (working copy)
>> @@ -50,4 +50,4 @@ c_parser_translation_unit (c_parser * pa
>>         }
>>      }
>>  }
>> -/* { dg-final { scan-tree-dump-not "0 != 0" "reassoc2"} } */
>> +/* { dg-final { scan-tree-dump-not "\[ (\]0 != 0" "reassoc2"} } */