Il 04/06/2012 11:31, Richard Guenther ha scritto: > + val = compare_range_with_value (NE_EXPR, vr, integer_zero_node, > &sop); > + if (!val || !integer_onep (val)) > + return false; > > please add a value_range_nonzero_p helper alongside value_range_nonnegative_p. > > + fndecl = builtin_decl_implicit (target_builtin_code); > + lhs = gimple_call_lhs (stmt); > + gcc_assert (TREE_TYPE (TREE_TYPE (fndecl)) == TREE_TYPE (lhs)); > > eh, if you care to check this please fail instead of ICEing ...
The verifier would fail very soon anyway. I might as well remove the assertion altogether. > Do we always have CTZ if we have FFS? Can't there be a target that > implements FFS as opcode but not CTZ, so you'd slow down things? > Thus, should the transform be conditonal on target support for CTZ > or no target support for FFS? Hmm, SH and (some semi-obscure variant of) SPARC. But actually SPARC should define a clz pattern instead; SH should have a popcount pattern + a generic trick to expand ctz/ffs in terms of popcount. I'll submit those before applying this patch. > Please add a comment to the code as to what transform you are doing > here. > > + /* Convert argument type. */ > + argtype = TREE_VALUE (TYPE_ARG_TYPES (TREE_TYPE (fndecl))); > + tem = create_tmp_reg (argtype, NULL); > + newop = gimple_build_assign_with_ops (NOP_EXPR, tem, op0, NULL_TREE); > + tem = make_ssa_name (tem, newop); > + gimple_assign_set_lhs (newop, tem); > + gsi_insert_before (gsi, newop, GSI_SAME_STMT); > > why is that necessary? Argument checking for GIMPLE_CALL is almost nonexistent, but I would like to be nice and create my calls with good arguments. > Can you at least wrap it inside a > > if (!useless_type_conversion_p (argtype, TREE_TYPE (op0))) > > ? Yes. Thanks for the review! Paolo
