On Wed, Oct 26, 2016 at 4:01 PM, Bin.Cheng <amker.ch...@gmail.com> wrote: > On Tue, Oct 25, 2016 at 1:00 PM, Richard Biener > <richard.guent...@gmail.com> wrote: >> On Tue, Oct 25, 2016 at 1:21 PM, Bin Cheng <bin.ch...@arm.com> wrote: >>> Hi, >>> This is an update patch for >>> https://gcc.gnu.org/ml/gcc-patches/2016-10/msg00738.html . In this >>> version, existing pattern (convert (op:s (convert@2 @0) (convert?@3 @1))) >>> is extended. It allows narrowing of arithmetic operation which has >>> constant integer as its second operand. It also simplifies next patch >>> handling cond_expr. >>> Bootstrap and test on x86_64 and AArch64 for whole patch set. Is it OK? >> >> + && types_match (@0, type) >> + && (types_match (@0, @1) >> + /* Or the second operand must be constant integer. */ >> + || (@3 == @1 >> + && types_match (@1, @2) >> + && TREE_CODE (@1) == INTEGER_CST))) >> >> So this fails to match the pattern if we get into it via valueization >> and get, say, >> (short)((int)a + (int)7). I believe for plus and minus we're always safe so >> I suggest to simply do >> >> && (types_match (@0, @1) >> || TREE_CODE (@1) == INTEGER_CST) >> >> (if (TYPE_OVERFLOW_WRAPS (TREE_TYPE (@0))) >> - (convert (op @0 @1)) >> + (convert (op @0 (convert:type @1))) >> >> :type shouldn't be necessary -- it also shows the outer (convert ..) >> is not required, >> please remove it while you're here. >> >> (with { tree utype = unsigned_type_for (TREE_TYPE (@0)); } >> - (convert (op (convert:utype @0) (convert:utype @1)))))))) >> + (convert (op (convert:utype @0) >> + (convert:utype (convert:type @1))))))))) >> >> Why do you need the intermediate conversion? > > Thanks for reviewing, updated patch attached. Is it OK?
Ok. Thanks, Richard. > Thanks, > bin
