On Tue, Nov 8, 2016 at 5:18 PM, Marc Glisse <marc.gli...@inria.fr> wrote: > On Tue, 8 Nov 2016, Dominik Vogt wrote: > >> On Fri, Nov 04, 2016 at 01:54:20PM +0100, Richard Biener wrote: >>> >>> On Fri, Nov 4, 2016 at 12:08 PM, Dominik Vogt <v...@linux.vnet.ibm.com> >>> wrote: >>>> >>>> On Fri, Nov 04, 2016 at 09:47:26AM +0100, Richard Biener wrote: >>>>> >>>>> On Thu, Nov 3, 2016 at 4:03 PM, Dominik Vogt <v...@linux.vnet.ibm.com> >>>>> wrote: >>>>>> >>>>>> Is VRP the right pass to do this optimisation or should a later >>>>>> pass rather attempt to eliminate the new use of b_5 instead? Uli >>>>>> has brought up the idea a mini "sign extend elimination" pass that >>>>>> checks if the result of a sign extend could be replaced by the >>>>>> original quantity in all places, and if so, eliminate the ssa >>>>>> name. (I guess that won't help with the above code because l is >>>>>> used also as a function argument.) How could a sensible approach >>>>>> to deal with the situation look like? >>>>> >>>>> >>>>> We run into this kind of situation regularly and for general foldings >>>>> in match.pd we settled with single_use () even though it is not >>>>> perfect. >>>>> Note the usual complaint is not extra extension instructions but >>>>> the increase of register pressure. >>>>> >>>>> This is because it is hard to do better when you are doing local >>>>> optimization. >>>>> >>>>> As for the question on whether VRP is the right pass to do this the >>>>> answer is two-fold -- VRP has the most precise range information. >>>>> But the folding itself should be moved to generic code and use >>>>> get_range_info (). >>>> >>>> >>>> All right, is this a sensible approach then? >>> >>> >>> Yes. >>> >>>> 1. Using has_single_use as in the experimental patch is good >>>> enough (provided further testing does not show serious >>>> regressions). >>> >>> >>> I'd approve that, yes. >>> >>>> 2. Rip the whole top level if-block from simplify_cond_using_ranges(). >>>> 3. Translate that code to match.pd syntax. >>> >>> >>> Might be some work but yes, that's also desired (you'd lose the ability >>> to emit the warnings though). >> >> >> Could you give me a match-pd-hint please? We now have something >> like this: >> >> (simplify >> (cond (gt SSA_NAME@0 INTEGER_CST@1) @2 @3) >> (if (... many conditions ...) >> (cond (gt ... ...) @2 @3)) >> >> The generated code ends up in gimple_simplify_COND_EXPR, but when >> gimple_simplify is actually called, it goes through the >> GIMPLE_COND case and calls gimple_resimplify2(..., GT, ...) and >> there it tries gimple_simplify_GT_EXPR(), peeling of the (cond >> ...), i.e. it never tries the generated code. > > > Not sure what you mean here. > >> There is another pattern in match.pd that uses a (cond ...) as the >> first operand, and I do not understand how this works. Should we >> just use "(gt SSA_NAME@0 INTEGER_CST@1)" as the first operand >> instead, and wouldn't this pattern be too general that way? > > > IIUC, you are trying to move the second half of simplify_cond_using_ranges > to match.pd. I don't see any reason to restrict it to the case where the > comparison result is used directly in a COND_EXPR, so that would look like: > > (for cmp (...) > (simplify > (cmp (convert SSA_NAME@0) INTEGER_CST@1) > (if (...) > (cmp @0 (convert @1))))) > > maybe? I think I may have missed your point.
Yeah, if you'd use (cond (gt ... then it only matches in assignments with COND_EXPRs on the RHS, _not_ in GIMPLE_CONDs. So you ran into the (cond vs. GIMPLE_COND "mismatch". You'd essentially implement sth similar to shorten_compare in match.pd. Btw, moving to match.pd shouldn't be a blocker for adding proper single_use tests just in case you get lost ... Richard. > (and yes, the first half would give a very general (simplify (cmp SSA_NAME@0 > INTEGER_CST@1) ...), that doesn't seem so bad) > > -- > Marc Glisse