On 11/07/14 22:47, Richard Biener wrote: > On Fri, Jul 11, 2014 at 1:52 PM, Kugan > wrote: >> Thanks foe the review and suggestions. >> >> On 10/07/14 22:15, Richard Biener wrote: >>> On Mon, Jul 7, 2014 at 8:55 AM, Kugan wrote: >> >> [...] >> >>>> >>>> For -fwrapv, it is due to how PROMOTE_MODE is defined in arm back-end. >>>> In the test-case, a function (which has signed char return type) returns >>>> -1 in one of the paths. ARM PROMOTE_MODE changes that to 255 and relies >>>> on zero/sign extension generated by RTL again for the correct value. I >>>> saw some other targets also defining similar think. I am therefore >>>> skipping removing zero/sign extension if the ssa variable can be set to >>>> negative integer constants. >>> >>> Hm? I think you should rather check that you are removing a >>> sign-/zero-extension - PROMOTE_MODE tells you if it will sign- or >>> zero-extend. Definitely >>> >>> + /* In some architectures, negative integer constants are truncated and >>> + sign changed with target defined PROMOTE_MODE macro. This will impact >>> + the value range seen here and produce wrong code if zero/sign extensions >>> + are eliminated. Therefore, return false if this SSA can have negative >>> + integers. */ >>> + if (is_gimple_assign (stmt) >>> + && (TREE_CODE_CLASS (gimple_assign_rhs_code (stmt)) == tcc_unary)) >>> + { >>> + tree rhs1 = gimple_assign_rhs1 (stmt); >>> + if (TREE_CODE (rhs1) == INTEGER_CST >>> + && !TYPE_UNSIGNED (TREE_TYPE (ssa)) >>> + && tree_int_cst_compare (rhs1, integer_zero_node) == -1) >>> + return false; >>> >>> looks completely bogus ... (an unary op with a constant operand?) >>> instead you want to do sth like >> >> I see that unary op with a constant operand is not possible in gimple. >> What I wanted to check here is any sort of constant loads; but seems >> that will not happen in gimple. Is PHI statements the only possible >> statements where we will end up with such constants. > > No, in theory you can have > > ssa_1 = -1; > > but that's not unary but a GIMPLE_SINGLE_RHS and thus > gimple_assign_rhs_code (stmt) == INTEGER_CST. > >>> mode = TYPE_MODE (TREE_TYPE (ssa)); >>> rhs_uns = TYPE_UNSIGNED (TREE_TYPE (ssa)); >>> PROMOTE_MODE (mode, rhs_uns, TREE_TYPE (ssa)); >>> >>> instead of initializing rhs_uns from ssas type. That is, if >>> PROMOTE_MODE tells you to promote _not_ according to ssas sign then >>> honor that. >> >> This is triggered in pr43017.c in function foo for arm-none-linux-gnueabi. >> >> where, the gimple statement that cause this looks like: >> ..... >> # _3 = PHI <_17(7), -1(2)> >> bb43: >> return _3; >> >> ARM PROMOTE_MODE changes the sign for integer constants only and hence >> looking at the variable with PROMOTE_MODE is not changing the sign in >> this case. >> >> #define PROMOTE_MODE(MODE, UNSIGNEDP, TYPE) \ >> if (GET_MODE_CLASS (MODE) == MODE_INT \ >> && GET_MODE_SIZE (MODE) < 4) \ >> { \ >> if (MODE == QImode) \ >> UNSIGNEDP = 1; \ >> else if (MODE == HImode) \ >> UNSIGNEDP = 1; \ >> (MODE) = SImode; \ >> } > > Where does it only apply for "constants"? It applies to all QImode and > HImode entities. oops, sorry. I don’t know what I was thinking or looking at when I wrote that :( It indeed fixes my problems. Thanks for that. Here is the modified patch. Bootstrapped and regression tested for 86_64-unknown-linux-gnu and arm-none-linux-gnueabi with no new regressions. Is this OK? Thanks, Kugan gcc/ 2014-07-14 Kugan Vivekanandarajah * calls.c (precompute_arguments): Check is_promoted_for_type and set the promoted mode. (is_promoted_for_type): New function. (expand_expr_real_1): Check is_promoted_for_type and set the promoted mode. * expr.h (is_promoted_for_type): New function definition. * cfgexpand.c (expand_gimple_stmt_1): Call emit_move_insn if SUBREG is promoted with SRP_SIGNED_AND_UNSIGNED. gcc/testsuite 2014-07-14 Kugan Vivekanandarajah * gcc.dg/zero_sign_ext_test.c: New test.