On Wed, Oct 26, 2016 at 5:22 PM, Bin.Cheng <amker.ch...@gmail.com> wrote: > On Wed, Oct 26, 2016 at 4:05 PM, Marc Glisse <marc.gli...@inria.fr> wrote: >> On Wed, 26 Oct 2016, Bin.Cheng wrote: >> >>> On Wed, Oct 26, 2016 at 3:10 PM, Bin.Cheng <amker.ch...@gmail.com> wrote: >>>> >>>> On Wed, Oct 26, 2016 at 3:04 PM, Marc Glisse <marc.gli...@inria.fr> >>>> wrote: >>>>> >>>>> On Wed, 26 Oct 2016, Bin.Cheng wrote: >>>>> >>>>>> Thanks for reviewing, updated patch attached. Is it OK? >>>>> >>>>> >>>>> >>>>> +/* (convert (minmax ((convert (x) c)))) -> minmax (x c) if x is >>>>> promoted >>>>> + and the outer convert demotes the expression back to x's type. */ >>>>> +(for minmax (min max) >>>>> + (simplify >>>>> + (convert (minmax@0 (convert @1) INTEGER_CST@2)) >>>>> + (if (types_match (@1, type) && int_fits_type_p (@2, type) >>>>> + && TYPE_PRECISION (TREE_TYPE (@0)) > TYPE_PRECISION (TREE_TYPE >>>>> (@1))) >>>>> + (minmax @1 (convert @2))))) >>>>> >>>>> Don't you have a problem if @1 is signed but not @0? >>>>> (int)max((unsigned long)(-2),3ul) >>>>> seems to satisfy your conditions, but is not the same as >>>>> max(-2,3) >>>> >>>> Ah, yes. I falsely removed sign check from the original patch. Will >>>> update that. >>>> >>> Here it is. Sorry for the mistake. >> >> >> I expect the issues are only with signed @1 and unsigned @0, the reverse >> should be safe. But conservatively requiring the same TYPE_SIGN works if you >> think the that covers enough cases. > It covers the motivation test case, but relaxed condition is of course > more useful. I will address this in followup patch extending this > pattern for non-const @2. Does this make sense?
Yes. The patch is ok. Thanks, Richard. > Thanks, > bin
