Richard Guenther <rguent...@suse.de> wrote on 02/08/2011 04:25:58 PM:
> > Thinking about it it probably makes sense to keep a variant of this > in the vectorizer - after all it has quite specific requirements on > operand sizes while VRP would probably demote as far as possible > (maybe taking PROMOTE_MODE into account). > > A quick look at your patch reveals > > + if (gimple_assign_rhs_code (use_stmt) == CONVERT_EXPR) > > CONVERT_EXPR_CODE_P (gimple_assign_rhs_code (use_stmt)) > > + tmp = create_tmp_var (use_type, NULL); > > create_tmp_reg Why? USE_TYPE is neither COMPLEX_TYPE nor VECTOR_TYPE. Thanks, Ira > > + if (!types_compatible_p (TREE_TYPE (oprnd0), type) > + || !types_compatible_p (TREE_TYPE (oprnd1), type) > + || (TREE_CODE (oprnd0) != INTEGER_CST > + && TREE_CODE (oprnd1) != INTEGER_CST)) > > it's always the second operand that is constant, you can simplify > the code to not handle CST op SSA. > > + code = gimple_assign_rhs_code (stmt); > + if (code != LSHIFT_EXPR && code != RSHIFT_EXPR > + && code != BIT_IOR_EXPR && code != BIT_XOR_EXPR && code != > BIT_AND_EXPR) > + return false; > + > + oprnd0 = gimple_assign_rhs1 (stmt); > + oprnd1 = gimple_assign_rhs2 (stmt); > + type = gimple_expr_type (stmt); > + if (!types_compatible_p (TREE_TYPE (oprnd0), type) > + || !types_compatible_p (TREE_TYPE (oprnd1), type) > > for shifts the type compatibility check of oprnd1 isn't guaranteed > (but do we care? we only will handle constant shift amounts), for > the other operands of the codes you handle they always return true. > > So I'd simplify the check to > > if (TREE_CODE (oprnd0) != SSA_NAME > || TREE_CODE (oprnd1) != INTEGER_CST) > return false; > > Otherwise the patch looks sensible. > > Richard.
