On Thu, May 16, 2013 at 12:05 PM, Marc Glisse <marc.gli...@inria.fr> wrote: > On Thu, 16 May 2013, Richard Biener wrote: > >> On Thu, May 16, 2013 at 8:42 AM, Marc Glisse <marc.gli...@inria.fr> wrote: >>> >>> Hello, >>> >>> we can get into a cycle where: >>> (x<0)|1 becomes (x<0)?-1:1 >>> and >>> (y?-1:1) becomes y|1 >>> >>> Contrary to what I posted in the PR, I am disabling the second >>> transformation here. It can be done later (the x86 target partially does >>> it >>> in the vcond expansion), and the VEC_COND_EXPR allows us to perform >>> further >>> operations: >>> (((x<0)|1)*5-1)/2 becomes (untested) (x<0)?-3:2 >>> >>> Also, this is a partial revert of the last patch, which sounds safer. I >>> am >>> leaving the vector tests in the disabled transformations in case we >>> decide >>> to re-enable them later. >>> >>> This isn't the end of the story, even for fold-const.c. I kept the >>> transformation a?-1:0 -> a, but it does not work because: >>> >>> /* If we try to convert OP0 to our type, the >>> call to fold will try to move the conversion inside >>> a COND, which will recurse. In that case, the COND_EXPR >>> is probably the best choice, so leave it alone. */ >>> && type == TREE_TYPE (arg0)) >>> >>> and when a is a comparison, its type is a different type (even looking >>> through main variant and canonical), only useless_type_conversion_p >>> notices >>> they are so similar, and I would still need a conversion, otherwise the >>> front-end complains when I try to assign the result that it has a >>> different >>> type than the variable I want to assign it to (I expected the result of >>> the >>> comparison to be opaque, and thus no complaining, but apparently not). >> >> >> Which is why most of these non-trivial transforms should happen on GIMPLE > > > Indeed, I guess I'll try that instead of risking more infinite loops, > thanks. Although when a transformation exists in fold-const.c, it is always > tempting to adapt it instead of rewriting it elsewhere in the compiler > (where it ends up 3 times as long and complicated)... > > >> via what I and Andrew proposed some year(s) ago. But well ... > > > You mean the existing tree-ssa-forwprop.c, or something different? > (I remember the valueize idea)
Basically what tree-ssa-forwprop.c does but wrapped in a fold-const like interface. See http://gcc.gnu.org/ml/gcc-patches/2011-03/msg01099.html, Andrew was having a branch somewhen. Richard. > >>> Also, we may want to make fold_binary_op_with_conditional_arg more strict >>> on >>> how much folding is necessary to consider the transformation worth it. >>> For >>> VEC_COND_EXPR where both branches are evaluated anyway, at least if we >>> started from a comparison and not already a VEC_COND_EXPR, we could >>> require >>> that both branches fold. >>> >>> But it seems better to fix the ICE quickly and do the rest later. >>> >>> Passes bootstrap+testsuite on x86_64-linux-gnu. >> >> >> Ok. >> >> Thanks, >> Richard. >> >>> 2013-05-16 Marc Glisse <marc.gli...@inria.fr> >>> >>> PR middle-end/57286 >>> gcc/ >>> * fold-const.c (fold_ternary_loc) <VEC_COND_EXPR>: Disable some >>> transformations to avoid an infinite loop. >>> >>> gcc/testsuite/ >>> * gcc.dg/pr57286.c: New testcase. >>> * gcc.dg/vector-shift-2.c: Don't assume int has size 4. >>> * g++.dg/ext/vector22.C: Comment out transformations not >>> performed anymore. > > > -- > Marc Glisse
