On Mon, Sep 2, 2013 at 11:46 AM, Marc Glisse <marc.gli...@inria.fr> wrote: > On Fri, 30 Aug 2013, Richard Biener wrote: > >> On Sat, Jul 6, 2013 at 9:42 PM, Marc Glisse <marc.gli...@inria.fr> wrote: >>> >>> First, the fold-const bit causes an assertion failure (building libjava) >>> in >>> combine_cond_expr_cond, which calls: >>> >>> t = fold_binary_loc (gimple_location (stmt), code, type, op0, op1); >>> >>> and then checks: >>> >>> /* Require that we got a boolean type out if we put one in. */ >>> gcc_assert (TREE_CODE (TREE_TYPE (t)) == TREE_CODE (type)); >>> >>> which makes sense... Note that the 2 relevant types are: >> >> >> well, the check is probably old and can be relaxed to also allow all >> compatible types. > > > Ok. Maybe it could even be removed then, we shouldn't check in every > function that calls fold_binary_loc that it returns something of the type > that was asked for. > > >>> Second, the way the forwprop transformation is done, it can be necessary >>> to >>> run the forwprop pass several times in a row, which is not nice. It >>> takes: >>> >>> stmt_cond >>> stmt_binop >>> >>> and produces: >>> >>> stmt_folded >>> stmt_cond2 >>> >>> But one of the arguments of stmt_folded could be an ssa_name defined by a >>> cond_expr, which could now be propagated into stmt_folded (there may be >>> other new possible transformations). However, that cond_expr has already >>> been visited and won't be again. The combine part of the pass uses a PLF >>> to >>> revisit statements, but the forwprop part doesn't have any specific >>> mechanism. >> >> >> forwprop is designed to re-process stmts it has folded to catch cascading >> effects. Now, as it as both a forward and a backward run you don't catch >> 2nd-order effects with iterating them. On my TODO is to only do one kind, >> either forward or backward propagation. > > > My impression is that even internally in the forward part, the > reprocessing doesn't really work, but that'll disappear anyway when the > forward part disappears. > > >> Btw, as for the patch I don't like that you basically feed everything into >> fold. Yes, I know we do that for conditions because that's quite >> important >> and nobody has re-written the condition folding as gimple pattern matcher. >> I doubt that this transform is important enough to warrant another >> exception ;) > > > I am not sure what you want here. When I notice the pattern (a?b:c) op d, I > need to check whether b op d and c op d are likely to simplify before > transforming it to a?(b op d):(c op d). And currently I can't see any way to > do that other than feeding (b op d) to fold. Even if/when we get a gimple > folder, we will still need to call it in about the same way.
With a gimple folder you'd avoid building trees. You are testing for a quite complex pattern here - (a?b:c) & (d?e:f). It seems to be that for example + vec c=(a>3)?o:z; + vec d=(b>2)?o:z; + vec e=c&d; would be better suited in the combine phase (you are interested in combining the comparisons). That is, look for a [&|^] b where a and b are [VEC_]COND_EXPRs with the same values. Heh, and it's not obvious to me with looking for a minute what this should be simplified to ... (so the code and the testcase needs some comments on what you are trying to catch ...) Richard. > -- > Marc Glisse
