[-ftree-cselim desoptimizes SH code

[Richard Guenther]

On Fri, Sep 25, 2009 at 1:21 PM, Christian BRUEL <christian.bruel@st.com> wrote:
> Hello,
>
> On SH, -ftree-cselim transforms a simple conditional store such as
>
> foo(int *a)
> {
>    *a = 0;
>  if (bar())
>    *a = 1;
> }
>
> into
>
> foo(int *a)
> {
>    *a = 0;
>  if (! bar())
>    r = *a;
>  else
>    r = 1;
>
>   *a = r;
> }
>
> which is equivalent to add either a load or a store on the critical path
> (depending if the branch is likely to be taken or not)
>
> more important, those new memory operations are not cleaned up, for example
> the SH code is looks like
>
> (-O2 -fno-schedule-insns -fno-schedule-insns2 -fno-delayed-branch
> -fomit-frame-pointer) to make the code more readable
>
>        tst     r0,r0
>        mov     #1,r1
>        bf      .L2
>        mov.l   @r8,r1
> .L2:
>        mov.l   r1,@r8
>        ...
>
> with -fno-tree-cselim I see the expected
>
>        tst     r0,r0
>        bt      .L3
>        mov     #1,r1
>        mov.l   r1,@r8
> .L3:
>        ...
>
> which is one memory load better and possibly one store better.
>
> Am I right in saying that this option is only useful if conditional
> execution (predication or select) is available (because the conditional is
> made explicit in the phi) ?

Well, yes.  The pass tries to uncover conditional store operations,
so if ifcvt does not have any means to create them (which IIRC is
more than just in case of HAVE_conditional_execution), then
the pass might not have any benefit.

Richard.

 so a guard on the gate such as p1.patch should
> be ok.
>
> or is it just an SH problem, so this option should only be disabled in the
> sh_optimization_option such as with p2.patch ?
>
> any feedback for the others targets ?
>
> many thanks
>
> Christian
>
>
>
>
>
> Index: tree-ssa-phiopt.c
> ===================================================================
> --- tree-ssa-phiopt.c   (revision 151937)
> +++ tree-ssa-phiopt.c   (working copy)
> @@ -1302,7 +1302,11 @@
>  static bool
>  gate_cselim (void)
>  {
> -  return flag_tree_cselim;
> +#ifdef HAVE_conditional_execution
> +  return flag_tree_cselim ;
> +#else
> +  return 0;
> +#endif
>  }
>
>  struct gimple_opt_pass pass_cselim =
>
> Index: config/sh/sh.c
> ===================================================================
> --- config/sh/sh.c      (revision 151937)
> +++ config/sh/sh.c      (working copy)
> @@ -674,6 +674,7 @@
>   if (flag_schedule_insns > 0)
>     flag_schedule_insns = 2;
>
> +  flag_tree_cselim = 0;
>   set_param_value ("simultaneous-prefetches", 2);
>  }
>
>
>