This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH] Builtins handling in IVOPT
- From: Wei Mi <wmi at google dot com>
- To: Zdenek Dvorak <rakdver at iuuk dot mff dot cuni dot cz>
- Cc: GCC Patches <gcc-patches at gcc dot gnu dot org>, Richard Biener <richard dot guenther at gmail dot com>, David Li <davidxl at google dot com>, amker cheng <amker dot cheng at gmail dot com>
- Date: Tue, 17 Dec 2013 11:34:02 -0800
- Subject: Re: [PATCH] Builtins handling in IVOPT
- Authentication-results: sourceware.org; auth=none
- References: <CA+4CFy7_1JO7r3eq8TWzotR1L6oGKkecZx24OvBd8yeXKcshWQ at mail dot gmail dot com> <20131121081913 dot GB23315 at kam dot mff dot cuni dot cz> <CA+4CFy5z=ZJSxZedadaxamsOuX0D4mOiiLG9OWDsoWkrY0Yhkg at mail dot gmail dot com> <CA+4CFy5bh80nL6w=3cKay68ACMk1w9eyOp_ELNfub-Q72OLhjQ at mail dot gmail dot com> <CA+4CFy6MDw5ybsrVM2fwsqH45LQ4uLgn5035eszrYHQSa1FiZA at mail dot gmail dot com>
Ping.
Thanks,
Wei.
On Mon, Dec 9, 2013 at 9:54 PM, Wei Mi <wmi@google.com> wrote:
> Ping.
>
> Thanks,
> wei.
>
> On Sat, Nov 23, 2013 at 10:46 AM, Wei Mi <wmi@google.com> wrote:
>> bootstrap and regression of the updated patch pass.
>>
>> On Sat, Nov 23, 2013 at 12:05 AM, Wei Mi <wmi@google.com> wrote:
>>> On Thu, Nov 21, 2013 at 12:19 AM, Zdenek Dvorak
>>> <rakdver@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 <wmi@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 <wmi@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