This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] Builtins handling in IVOPT


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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]