On Thu, 11 Feb 2016, Christophe Lyon wrote:
> On 9 February 2016 at 13:42, Richard Biener <rguent...@suse.de> wrote:
> >
> > It turns out if-conversions poor job on
> >
> > if (a)
> > x[i] = ...;
> > else
> > x[i] = ...;
> >
> > results in bogus uninit warnings of x[i] for a variety of reasons.
> > First of all forwprop (aka match.pd simplification) doesn't fixup
> > all of if-conversions poor job as canonicalization sometimes
> > inverts the condition in [VEC_]COND_EXPRs and thus the existing
> > A ? B : (A ? X : C) -> A ? B : C pattern doesn't apply. The match.pd
> > hunk fixes this (albeit in an awkward way - I don't feel like mucking
> > with genmatch at this stage, nor exactly for the poor [VEC_]COND_EXPR
> > IL we should rather fix). Second, the late uninit pass is confused
> > by the left-over dead code, in this case dead load feeding a dead
> > VEC_COND_EXPR. Adding a DCE pass before late uninit as the comment
> > in passes.def suggests fixes this and also should avoid creating the dead
> > RTL I've sometimes seen.
> >
> > Due to the PR69719 fix we're now over the alias-test limit for the
> > testcase (well, all alias tests are bogus, see PR69732), so I upped
> > that limit for the testcase. I'm investigating the Job done there.
> >
> > Bootstrap and regtest is currently running on x86_64-unknown-linux-gnu.
> >
> Hi,
>
> After this patch, I noticed that
> g++.dg/tree-ssa/pr61034.C -std=gnu++11 scan-tree-dump-times
> optimized "free" 4
> now fails on arm* and aarch64*
> The dump contains no occurrence of "free".
>
> Is this just a matter of adjusting the test-case or an unexpected
> problem with this patch?
Yeah, I fixed the testcase up this morning.
Richard.
> Christophe.
>
> > Richard.
> >
> > 2016-02-09 Richard Biener <rguent...@suse.de>
> >
> > PR tree-optimization/69726
> > * passes.def: Add DCE pass before late uninit.
> > * match.pd: Add A ? B : (!A ? C : X) -> A ? B : C patterns to
> > really fixup if-conversions job.
> >
> > * gcc.dg/uninit-22.c: New testcase.
> >
> > Index: gcc/passes.def
> > ===================================================================
> > *** gcc/passes.def (revision 233241)
> > --- gcc/passes.def (working copy)
> > *************** along with GCC; see the file COPYING3.
> > *** 322,336 ****
> > NEXT_PASS (pass_fold_builtins);
> > NEXT_PASS (pass_optimize_widening_mul);
> > NEXT_PASS (pass_tail_calls);
> > ! /* FIXME: If DCE is not run before checking for uninitialized uses,
> > we may get false warnings (e.g., testsuite/gcc.dg/uninit-5.c).
> > However, this also causes us to misdiagnose cases that should be
> > ! real warnings (e.g., testsuite/gcc.dg/pr18501.c).
> > !
> > ! To fix the false positives in uninit-5.c, we would have to
> > ! account for the predicates protecting the set and the use of each
> > ! variable. Using a representation like Gated Single Assignment
> > ! may help. */
> > /* Split critical edges before late uninit warning to reduce the
> > number of false positives from it. */
> > NEXT_PASS (pass_split_crit_edges);
> > --- 322,332 ----
> > NEXT_PASS (pass_fold_builtins);
> > NEXT_PASS (pass_optimize_widening_mul);
> > NEXT_PASS (pass_tail_calls);
> > ! /* If DCE is not run before checking for uninitialized uses,
> > we may get false warnings (e.g., testsuite/gcc.dg/uninit-5.c).
> > However, this also causes us to misdiagnose cases that should be
> > ! real warnings (e.g., testsuite/gcc.dg/pr18501.c). */
> > ! NEXT_PASS (pass_dce);
> > /* Split critical edges before late uninit warning to reduce the
> > number of false positives from it. */
> > NEXT_PASS (pass_split_crit_edges);
> > Index: gcc/match.pd
> > ===================================================================
> > *** gcc/match.pd (revision 233241)
> > --- gcc/match.pd (working copy)
> > *************** DEFINE_INT_AND_FLOAT_ROUND_FN (RINT)
> > *** 1717,1722 ****
> > --- 1717,1745 ----
> > (simplify
> > (cnd @0 @1 (cnd @0 @2 @3))
> > (cnd @0 @1 @3))
> > + /* A ? B : (!A ? C : X) -> A ? B : C. */
> > + /* ??? This matches embedded conditions open-coded because genmatch
> > + would generate matching code for conditions in separate stmts only.
> > + The following is still important to merge then and else arm cases
> > + from if-conversion. */
> > + (simplify
> > + (cnd @0 @1 (cnd @2 @3 @4))
> > + (if (COMPARISON_CLASS_P (@0)
> > + && COMPARISON_CLASS_P (@2)
> > + && invert_tree_comparison
> > + (TREE_CODE (@0), HONOR_NANS (TREE_OPERAND (@0, 0))) ==
> > TREE_CODE (@2)
> > + && operand_equal_p (TREE_OPERAND (@0, 0), TREE_OPERAND (@2, 0), 0)
> > + && operand_equal_p (TREE_OPERAND (@0, 1), TREE_OPERAND (@2, 1), 0))
> > + (cnd @0 @1 @3)))
> > + (simplify
> > + (cnd @0 (cnd @1 @2 @3) @4)
> > + (if (COMPARISON_CLASS_P (@0)
> > + && COMPARISON_CLASS_P (@1)
> > + && invert_tree_comparison
> > + (TREE_CODE (@0), HONOR_NANS (TREE_OPERAND (@0, 0))) ==
> > TREE_CODE (@1)
> > + && operand_equal_p (TREE_OPERAND (@0, 0), TREE_OPERAND (@1, 0), 0)
> > + && operand_equal_p (TREE_OPERAND (@0, 1), TREE_OPERAND (@1, 1), 0))
> > + (cnd @0 @3 @4)))
> >
> > /* A ? B : B -> B. */
> > (simplify
> > Index: gcc/testsuite/gcc.dg/uninit-22.c
> > ===================================================================
> > *** gcc/testsuite/gcc.dg/uninit-22.c (revision 0)
> > --- gcc/testsuite/gcc.dg/uninit-22.c (revision 0)
> > ***************
> > *** 0 ****
> > --- 1,69 ----
> > + /* { dg-do compile } */
> > + /* { dg-options "-O3 -Wuninitialized --param
> > vect-max-version-for-alias-checks=20" } */
> > +
> > + #include <stdint.h>
> > +
> > + #define A1 2896 /* (1/sqrt(2))<<12 */
> > + #define A2 2217
> > + #define A3 3784
> > + #define A4 -5352
> > +
> > + #define
> > IDCT_TRANSFORM(dest,s0,s1,s2,s3,s4,s5,s6,s7,d0,d1,d2,d3,d4,d5,d6,d7,munge,src)
> > {\
> > + const int a0 = (src)[s0] + (src)[s4]; \
> > + const int a1 = (src)[s0] - (src)[s4]; \
> > + const int a2 = (src)[s2] + (src)[s6]; \
> > + const int a3 = (A1*((src)[s2] - (src)[s6])) >> 11; \
> > + const int a4 = (src)[s5] + (src)[s3]; \
> > + const int a5 = (src)[s5] - (src)[s3]; \
> > + const int a6 = (src)[s1] + (src)[s7]; \
> > + const int a7 = (src)[s1] - (src)[s7]; \
> > + const int b0 = a4 + a6; \
> > + const int b1 = (A3*(a5 + a7)) >> 11; \
> > + const int b2 = ((A4*a5) >> 11) - b0 + b1; \
> > + const int b3 = (A1*(a6 - a4) >> 11) - b2; \
> > + const int b4 = ((A2*a7) >> 11) + b3 - b1; \
> > + (dest)[d0] = munge(a0+a2 +b0); \
> > + (dest)[d1] = munge(a1+a3-a2+b2); \
> > + (dest)[d2] = munge(a1-a3+a2+b3); \
> > + (dest)[d3] = munge(a0-a2 -b4); \
> > + (dest)[d4] = munge(a0-a2 +b4); \
> > + (dest)[d5] = munge(a1-a3+a2-b3); \
> > + (dest)[d6] = munge(a1+a3-a2-b2); \
> > + (dest)[d7] = munge(a0+a2 -b0); \
> > + }
> > +
> > + #define MUNGE_NONE(x) (x)
> > + #define IDCT_COL(dest,src)
> > IDCT_TRANSFORM(dest,0,8,16,24,32,40,48,56,0,8,16,24,32,40,48,56,MUNGE_NONE,src)
> > +
> > + #define MUNGE_ROW(x) (((x) + 0x7F)>>8)
> > + #define IDCT_ROW(dest,src)
> > IDCT_TRANSFORM(dest,0,1,2,3,4,5,6,7,0,1,2,3,4,5,6,7,MUNGE_ROW,src)
> > +
> > + static inline void bink_idct_col(int *dest, const int32_t *src)
> > + {
> > + if ((src[8]|src[16]|src[24]|src[32]|src[40]|src[48]|src[56])==0) {
> > + dest[0] =
> > + dest[8] =
> > + dest[16] =
> > + dest[24] =
> > + dest[32] =
> > + dest[40] =
> > + dest[48] =
> > + dest[56] = src[0];
> > + } else {
> > + IDCT_COL(dest, src);
> > + }
> > + }
> > +
> > + int bink_idct_put_c(uint8_t *dest, int linesize, int32_t *block)
> > + {
> > + int i;
> > + int temp[64];
> > + for (i = 0; i < 8; i++)
> > + bink_idct_col(&temp[i], &block[i]);
> > + for (i = 0; i < 8; i++) {
> > + IDCT_ROW( (&dest[i*linesize]), (&temp[8*i]) );
> > + }
> > +
> > + return 0;
> > + }
> > +
>
>
--
Richard Biener <rguent...@suse.de>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB
21284 (AG Nuernberg)
