bootstrap and regression of the updated patch pass.
On Sat, Nov 23, 2013 at 12:05 AM, Wei Mi <w...@google.com> wrote: > On Thu, Nov 21, 2013 at 12:19 AM, Zdenek Dvorak > <rakd...@iuuk.mff.cuni.cz> wrote: >> Hi, >> >>> This patch works on the intrinsic calls handling issue in IVOPT mentioned >>> here: >>> http://gcc.gnu.org/ml/gcc-patches/2010-10/msg01295.html >>> >>> In find_interesting_uses_stmt, it changes >>> >>> arg = expr >>> __builtin_xxx (arg) >>> >>> to >>> >>> arg = expr; >>> tmp = addr_expr (mem_ref(arg)); >>> __builtin_xxx (tmp, ...) >> >> this looks a bit confusing (and wasteful) to me. It would make more sense to >> just record the argument as USE_ADDRESS and do the rewriting in >> rewrite_use_address. >> >> Zdenek > > I updated the patch. The gimple changing part is now moved to > rewrite_use_address. Add support for plain address expr in addition to > reference expr in find_interesting_uses_address. > > bootstrap and testing is going on. > > 2013-11-22 Wei Mi <w...@google.com> > > * expr.c (expand_expr_addr_expr_1): Not to split TMR. > (expand_expr_real_1): Ditto. > * targhooks.c (default_builtin_has_mem_ref_p): Default > builtin. > * tree-ssa-loop-ivopts.c (builtin_has_mem_ref_p): New function. > (rewrite_use_address): Add TMR for builtin. > (find_interesting_uses_stmt): Special handling of builtins. > * gimple-expr.c (is_gimple_address): Add handling of TMR. > * gimple-expr.h (is_gimple_addressable): Ditto. > * config/i386/i386.c (ix86_builtin_has_mem_ref_p): New target hook. > (ix86_atomic_assign_expand_fenv): Ditto. > (ix86_expand_special_args_builtin): Special handling of TMR for > builtin. > * target.def (builtin_has_mem_ref_p): New hook. > * doc/tm.texi.in: Ditto. > * doc/tm.texi: Generated. > > 2013-11-22 Wei Mi <w...@google.com> > > * gcc.dg/tree-ssa/ivopt_5.c: New test. > > Index: testsuite/gcc.dg/tree-ssa/ivopt_5.c > =================================================================== > --- testsuite/gcc.dg/tree-ssa/ivopt_5.c (revision 0) > +++ testsuite/gcc.dg/tree-ssa/ivopt_5.c (revision 0) > @@ -0,0 +1,21 @@ > +/* { dg-do compile { target {{ i?86-*-* x86_64-*-* } && lp64 } } } */ > +/* { dg-options "-O2 -m64 -fdump-tree-ivopts-details" } */ > + > +/* Make sure only one iv is selected after IVOPT. */ > + > +#include <x86intrin.h> > +extern __m128i arr[], d[]; > +void test (void) > +{ > + unsigned int b; > + for (b = 0; b < 1000; b += 2) { > + __m128i *p = (__m128i *)(&d[b]); > + __m128i a = _mm_load_si128(&arr[4*b+3]); > + __m128i v = _mm_loadu_si128(p); > + v = _mm_xor_si128(v, a); > + _mm_storeu_si128(p, v); > + } > +} > + > +/* { dg-final { scan-tree-dump-times "PHI <ivtmp" 1 "ivopts"} } */ > +/* { dg-final { cleanup-tree-dump "ivopts" } } */ > Index: targhooks.c > =================================================================== > --- targhooks.c (revision 204792) > +++ targhooks.c (working copy) > @@ -566,6 +566,13 @@ default_builtin_reciprocal (unsigned int > } > > bool > +default_builtin_has_mem_ref_p (int built_in_function ATTRIBUTE_UNUSED, > + int i ATTRIBUTE_UNUSED) > +{ > + return false; > +} > + > +bool > hook_bool_CUMULATIVE_ARGS_mode_tree_bool_false ( > cumulative_args_t ca ATTRIBUTE_UNUSED, > enum machine_mode mode ATTRIBUTE_UNUSED, > Index: expr.c > =================================================================== > --- expr.c (revision 204792) > +++ expr.c (working copy) > @@ -7467,7 +7467,19 @@ expand_expr_addr_expr_1 (tree exp, rtx t > tem = fold_build_pointer_plus (tem, TREE_OPERAND (exp, 1)); > return expand_expr (tem, target, tmode, modifier); > } > + case TARGET_MEM_REF: > + { > + int old_cse_not_expected; > + addr_space_t as > + = TYPE_ADDR_SPACE (TREE_TYPE (TREE_TYPE (TREE_OPERAND (exp, 0)))); > > + result = addr_for_mem_ref (exp, as, true); > + old_cse_not_expected = cse_not_expected; > + cse_not_expected = true; > + result = memory_address_addr_space (tmode, result, as); > + cse_not_expected = old_cse_not_expected; > + return result; > + } > case CONST_DECL: > /* Expand the initializer like constants above. */ > result = XEXP (expand_expr_constant (DECL_INITIAL (exp), > @@ -9526,9 +9538,13 @@ expand_expr_real_1 (tree exp, rtx target > = TYPE_ADDR_SPACE (TREE_TYPE (TREE_TYPE (TREE_OPERAND (exp, 0)))); > enum insn_code icode; > unsigned int align; > + int old_cse_not_expected; > > op0 = addr_for_mem_ref (exp, as, true); > + old_cse_not_expected = cse_not_expected; > + cse_not_expected = true; > op0 = memory_address_addr_space (mode, op0, as); > + cse_not_expected = old_cse_not_expected; > temp = gen_rtx_MEM (mode, op0); > set_mem_attributes (temp, exp, 0); > set_mem_addr_space (temp, as); > Index: gimple-expr.c > =================================================================== > --- gimple-expr.c (revision 204792) > +++ gimple-expr.c (working copy) > @@ -633,7 +633,8 @@ is_gimple_address (const_tree t) > op = TREE_OPERAND (op, 0); > } > > - if (CONSTANT_CLASS_P (op) || TREE_CODE (op) == MEM_REF) > + if (CONSTANT_CLASS_P (op) || TREE_CODE (op) == MEM_REF > + || TREE_CODE (op) == TARGET_MEM_REF) > return true; > > switch (TREE_CODE (op)) > Index: gimple-expr.h > =================================================================== > --- gimple-expr.h (revision 204792) > +++ gimple-expr.h (working copy) > @@ -122,7 +122,8 @@ static inline bool > is_gimple_addressable (tree t) > { > return (is_gimple_id (t) || handled_component_p (t) > - || TREE_CODE (t) == MEM_REF); > + || TREE_CODE (t) == MEM_REF > + || TREE_CODE (t) == TARGET_MEM_REF); > } > > /* Return true if T is a valid gimple constant. */ > Index: target.def > =================================================================== > --- target.def (revision 204792) > +++ target.def (working copy) > @@ -1742,6 +1742,14 @@ HOOK_VECTOR_END (vectorize) > #undef HOOK_PREFIX > #define HOOK_PREFIX "TARGET_" > > +DEFHOOK > +(builtin_has_mem_ref_p, > + "This hook return whether the @var{i}th param of the > @var{builtin_function}\n\ > +is a memory reference. If @var{i} is -1, return whether the > @var{builtin_function}\n\ > +contains any memory reference type param.", > + bool, (int builtin_function, int i), > + default_builtin_has_mem_ref_p) > + > /* Allow target specific overriding of option settings after options have > been changed by an attribute or pragma or when it is reset at the > end of the code affected by an attribute or pragma. */ > Index: tree-ssa-loop-ivopts.c > =================================================================== > --- tree-ssa-loop-ivopts.c (revision 204792) > +++ tree-ssa-loop-ivopts.c (working copy) > @@ -1822,7 +1822,8 @@ find_interesting_uses_address (struct iv > > /* Check that the base expression is addressable. This needs > to be done after substituting bases of IVs into it. */ > - if (may_be_nonaddressable_p (base)) > + if (may_be_nonaddressable_p (base) > + && REFERENCE_CLASS_P (base)) > goto fail; > > /* Moreover, on strict alignment platforms, check that it is > @@ -1830,7 +1831,11 @@ find_interesting_uses_address (struct iv > if (STRICT_ALIGNMENT && may_be_unaligned_p (base, step)) > goto fail; > > - base = build_fold_addr_expr (base); > + /* If base is of reference class, build its addr expr here. If > + base is already an address, then don't need to build addr > + expr. */ > + if (REFERENCE_CLASS_P (base)) > + base = build_fold_addr_expr (base); > > /* Substituting bases of IVs into the base expression might > have caused folding opportunities. */ > @@ -1874,13 +1879,36 @@ find_invariants_stmt (struct ivopts_data > } > } > > +/* Find whether the Ith param of the BUILTIN is a mem > + reference. If I is -1, it returns whether the BUILTIN > + contains any mem reference type param. */ > + > +static bool > +builtin_has_mem_ref_p (gimple builtin, int i) > +{ > + tree fndecl = gimple_call_fndecl (builtin); > + if (DECL_BUILT_IN_CLASS (fndecl) == BUILT_IN_NORMAL) > + { > + switch (DECL_FUNCTION_CODE (fndecl)) > + { > + case BUILT_IN_PREFETCH: > + if (i == -1 || i == 0) > + return true; > + } > + } > + else if (DECL_BUILT_IN_CLASS (fndecl) == BUILT_IN_MD) > + return targetm.builtin_has_mem_ref_p ((int) DECL_FUNCTION_CODE > (fndecl), i); > + > + return false; > +} > + > /* Finds interesting uses of induction variables in the statement STMT. */ > > static void > find_interesting_uses_stmt (struct ivopts_data *data, gimple stmt) > { > struct iv *iv; > - tree op, *lhs, *rhs; > + tree op, *lhs, *rhs, callee; > ssa_op_iter iter; > use_operand_p use_p; > enum tree_code code; > @@ -1928,14 +1956,40 @@ find_interesting_uses_stmt (struct ivopt > find_interesting_uses_cond (data, stmt); > return; > } > + } > + /* Handle builtin call if it could be expanded to insn > + with memory access operands. Generate USE_ADDRESS type > + use for address expr which is used for memory access of > + such builtin. */ > + else if (is_gimple_call (stmt) > + && (callee = gimple_call_fndecl (stmt)) > + && is_builtin_fn (callee) > + && builtin_has_mem_ref_p (stmt, -1)) > + { > + size_t i; > + for (i = 0; i < gimple_call_num_args (stmt); i++) > + { > + if (builtin_has_mem_ref_p (stmt, i)) > + { > + gimple def; > + tree *arg = gimple_call_arg_ptr (stmt, i); > > - /* TODO -- we should also handle address uses of type > - > - memory = call (whatever); > - > - and > + if (TREE_CODE (*arg) != SSA_NAME) > + continue; > > - call (memory). */ > + def = SSA_NAME_DEF_STMT (*arg); > + gcc_assert (gimple_code (def) == GIMPLE_PHI > + || is_gimple_assign (def)); > + gcc_assert (POINTER_TYPE_P (TREE_TYPE (*arg))); > + find_interesting_uses_address (data, stmt, arg); > + } > + else > + { > + tree arg = gimple_call_arg (stmt, i); > + find_interesting_uses_op (data, arg); > + } > + } > + return; > } > > if (gimple_code (stmt) == GIMPLE_PHI > @@ -6360,7 +6414,7 @@ rewrite_use_address (struct ivopts_data > aff_tree aff; > gimple_stmt_iterator bsi = gsi_for_stmt (use->stmt); > tree base_hint = NULL_TREE; > - tree ref, iv; > + tree ref, iv, callee; > bool ok; > > adjust_iv_update_pos (cand, use); > @@ -6383,10 +6437,41 @@ rewrite_use_address (struct ivopts_data > base_hint = var_at_stmt (data->current_loop, cand, use->stmt); > > iv = var_at_stmt (data->current_loop, cand, use->stmt); > - ref = create_mem_ref (&bsi, TREE_TYPE (*use->op_p), &aff, > - reference_alias_ptr_type (*use->op_p), > - iv, base_hint, data->speed); > - copy_ref_info (ref, *use->op_p); > + > + /* For builtin_call(addr_expr), change it to: > + tmp = ADDR_EXPR(TMR(...)); > + call (tmp); */ > + if (is_gimple_call (use->stmt) > + && (callee = gimple_call_fndecl (use->stmt)) > + && is_builtin_fn (callee)) > + { > + gimple g; > + gimple_seq seq = NULL; > + tree addr; > + tree type = TREE_TYPE (TREE_TYPE (*use->op_p)); > + location_t loc = gimple_location (use->stmt); > + gimple_stmt_iterator gsi = gsi_for_stmt (use->stmt); > + > + ref = create_mem_ref (&bsi, type, &aff, > + TREE_TYPE (*use->op_p), > + iv, base_hint, data->speed); > + addr = build1 (ADDR_EXPR, TREE_TYPE (*use->op_p), ref); > + g = gimple_build_assign_with_ops (ADDR_EXPR, > + make_ssa_name (TREE_TYPE (*use->op_p), NULL), > + addr, NULL); > + gimple_set_location (g, loc); > + gimple_seq_add_stmt_without_update (&seq, g); > + gsi_insert_seq_before (&gsi, seq, GSI_SAME_STMT); > + > + ref = gimple_assign_lhs (g); > + } > + else > + { > + ref = create_mem_ref (&bsi, TREE_TYPE (*use->op_p), &aff, > + reference_alias_ptr_type (*use->op_p), > + iv, base_hint, data->speed); > + copy_ref_info (ref, *use->op_p); > + } > *use->op_p = ref; > } > > Index: config/i386/i386.c > =================================================================== > --- config/i386/i386.c (revision 204792) > +++ config/i386/i386.c (working copy) > @@ -29639,6 +29639,50 @@ ix86_init_mmx_sse_builtins (void) > } > } > > +/* Return whether the Ith param of the BUILTIN_FUNCTION > + is a memory reference. If I == -1, return whether the > + BUILTIN_FUNCTION contains any memory reference param. */ > + > +static bool > +ix86_builtin_has_mem_ref_p (int builtin_function, int i) > +{ > + switch ((enum ix86_builtins) builtin_function) > + { > + /* LOAD. */ > + case IX86_BUILTIN_LOADHPS: > + case IX86_BUILTIN_LOADLPS: > + case IX86_BUILTIN_LOADHPD: > + case IX86_BUILTIN_LOADLPD: > + if (i == -1 || i == 1) > + return true; > + break; > + case IX86_BUILTIN_LOADUPS: > + case IX86_BUILTIN_LOADUPD: > + case IX86_BUILTIN_LOADDQU: > + case IX86_BUILTIN_LOADUPD256: > + case IX86_BUILTIN_LOADUPS256: > + case IX86_BUILTIN_LOADDQU256: > + case IX86_BUILTIN_LDDQU256: > + if (i == -1 || i == 0) > + return true; > + break; > + /* STORE. */ > + case IX86_BUILTIN_STOREHPS: > + case IX86_BUILTIN_STORELPS: > + case IX86_BUILTIN_STOREUPS: > + case IX86_BUILTIN_STOREUPD: > + case IX86_BUILTIN_STOREDQU: > + case IX86_BUILTIN_STOREUPD256: > + case IX86_BUILTIN_STOREUPS256: > + case IX86_BUILTIN_STOREDQU256: > + if (i == -1 || i == 0) > + return true; > + default: > + break; > + } > + return false; > +} > + > /* This adds a condition to the basic_block NEW_BB in function FUNCTION_DECL > to return a pointer to VERSION_DECL if the outcome of the expression > formed by PREDICATE_CHAIN is true. This function will be called during > @@ -32525,7 +32569,13 @@ ix86_expand_special_args_builtin (const > if (i == memory) > { > /* This must be the memory operand. */ > - op = force_reg (Pmode, convert_to_mode (Pmode, op, 1)); > + > + /* We expect the builtin could be expanded to rtl with memory > + operand and proper addressing mode will be kept as specified > + in TARGET_MEM_REF. */ > + if (!(TREE_CODE (arg) == ADDR_EXPR > + && TREE_CODE (TREE_OPERAND (arg, 0)) == TARGET_MEM_REF)) > + op = force_reg (Pmode, convert_to_mode (Pmode, op, 1)); > op = gen_rtx_MEM (mode, op); > gcc_assert (GET_MODE (op) == mode > || GET_MODE (op) == VOIDmode); > @@ -43737,6 +43787,9 @@ ix86_atomic_assign_expand_fenv (tree *ho > #undef TARGET_BUILTIN_RECIPROCAL > #define TARGET_BUILTIN_RECIPROCAL ix86_builtin_reciprocal > > +#undef TARGET_BUILTIN_HAS_MEM_REF_P > +#define TARGET_BUILTIN_HAS_MEM_REF_P ix86_builtin_has_mem_ref_p > + > #undef TARGET_ASM_FUNCTION_EPILOGUE > #define TARGET_ASM_FUNCTION_EPILOGUE ix86_output_function_epilogue > > Index: doc/tm.texi > =================================================================== > --- doc/tm.texi (revision 204792) > +++ doc/tm.texi (working copy) > @@ -709,6 +709,12 @@ If a target implements string objects th > If a target implements string objects then this hook should should > provide a facility to check the function arguments in @var{args_list} > against the format specifiers in @var{format_arg} where the type of > @var{format_arg} is one recognized as a valid string reference type. > @end deftypefn > > +@deftypefn {Target Hook} bool TARGET_BUILTIN_HAS_MEM_REF_P (int > @var{builtin_function}, int @var{i}) > +This hook return whether the @var{i}th param of the @var{builtin_function} > +is a memory reference. If @var{i} is -1, return whether the > @var{builtin_function} > +contains any memory reference type param. > +@end deftypefn > + > @deftypefn {Target Hook} void TARGET_OVERRIDE_OPTIONS_AFTER_CHANGE (void) > This target function is similar to the hook @code{TARGET_OPTION_OVERRIDE} > but is called when the optimize level is changed via an attribute or > Index: doc/tm.texi.in > =================================================================== > --- doc/tm.texi.in (revision 204792) > +++ doc/tm.texi.in (working copy) > @@ -697,6 +697,8 @@ should use @code{TARGET_HANDLE_C_OPTION} > > @hook TARGET_CHECK_STRING_OBJECT_FORMAT_ARG > > +@hook TARGET_BUILTIN_HAS_MEM_REF_P > + > @hook TARGET_OVERRIDE_OPTIONS_AFTER_CHANGE > > @defmac C_COMMON_OVERRIDE_OPTIONS
