vectorization, scheduling and aliasing

vectorization, scheduling and aliasing

[roy rosen]

Hi All,

I have implemented some vectorization features in my gcc port.

In the generated code for this function I can see a scheduling problem:

int xxx(int* __restrict__ a, int* __restrict__ b)
{
    int __restrict__ i;
    for (i = 0; i < 8; i++)
    {
        a[i] = b[i];
    }
    return 0;
}

from asmcons:

(insn 17 14 18 3 a.c:15 (set (reg:V4HI 105)
        (mem:V4HI (reg/v/f:SI 100 [ b ]) [2 S8 A64])) 115 {*movv4hi_load} (nil))

(insn 18 17 19 3 a.c:15 (set (mem:V4HI (reg/v/f:SI 99 [ a ]) [2 S8 A64])
        (reg:V4HI 105)) 116 {*movv4hi_store} (expr_list:REG_DEAD (reg:V4HI 105)
        (nil)))

(insn 19 18 20 3 a.c:15 (set (reg:V4HI 106)
        (mem:V4HI (plus:SI (reg/v/f:SI 100 [ b ])
                (const_int 8 [0x8])) [2 S8 A64])) 115 {*movv4hi_load}
(expr_list:REG_DEAD (reg/v/f:SI 100 [ b ])
        (nil)))

(insn 20 19 58 3 a.c:15 (set (mem:V4HI (plus:SI (reg/v/f:SI 99 [ a ])
                (const_int 8 [0x8])) [2 S8 A64])
        (reg:V4HI 106)) 116 {*movv4hi_store} (expr_list:REG_DEAD (reg:V4HI 106)
        (expr_list:REG_DEAD (reg/v/f:SI 99 [ a ])
            (nil))))

 from sched1 - dependecies list:
   --- Region Dependences --- b 3 bb 0
;;      insn  code    bb   dep  prio  cost   reservation
;;      ----  ----    --   ---  ----  ----   -----------
;;       17   115     3     0     4     1   (2_slots+lsu)	: 58 20 18
;;       18   116     3     1     3     1   (2_slots+lsu)	: 58 19
;;       19   115     3     1     2     1   (2_slots+lsu)	: 58 20
;;       20   116     3     2     1     1   (2_slots+lsu)	: 58
;;       58   101     3     4     1     1   (3_slots+pcu)	:

As you can see, insn 19 is dependant on 18.
I think that this should not happen since the arrays has __restrict__.
It might have something to do with aliasing.

Are you aware of any kind of problems regarding aliasing and vectors?
Can you think of anything else that might be wrong in my code that
leads gcc to think that there is a dependency between insn 18 and 19?

Thanks, Roy.

Re: vectorization, scheduling and aliasing

[Richard Guenther]

On Wed, Apr 14, 2010 at 8:48 AM, roy rosen <roy.1rosen@gmail.com> wrote:
> Hi All,
>
> I have implemented some vectorization features in my gcc port.
>
> In the generated code for this function I can see a scheduling problem:
>
> int xxx(int* __restrict__ a, int* __restrict__ b)
> {
>    int __restrict__ i;
>    for (i = 0; i < 8; i++)
>    {
>        a[i] = b[i];
>    }
>    return 0;
> }
>
> from asmcons:
>
> (insn 17 14 18 3 a.c:15 (set (reg:V4HI 105)
>        (mem:V4HI (reg/v/f:SI 100 [ b ]) [2 S8 A64])) 115 {*movv4hi_load} (nil))
>
> (insn 18 17 19 3 a.c:15 (set (mem:V4HI (reg/v/f:SI 99 [ a ]) [2 S8 A64])
>        (reg:V4HI 105)) 116 {*movv4hi_store} (expr_list:REG_DEAD (reg:V4HI 105)
>        (nil)))
>
> (insn 19 18 20 3 a.c:15 (set (reg:V4HI 106)
>        (mem:V4HI (plus:SI (reg/v/f:SI 100 [ b ])
>                (const_int 8 [0x8])) [2 S8 A64])) 115 {*movv4hi_load}
> (expr_list:REG_DEAD (reg/v/f:SI 100 [ b ])
>        (nil)))
>
> (insn 20 19 58 3 a.c:15 (set (mem:V4HI (plus:SI (reg/v/f:SI 99 [ a ])
>                (const_int 8 [0x8])) [2 S8 A64])
>        (reg:V4HI 106)) 116 {*movv4hi_store} (expr_list:REG_DEAD (reg:V4HI 106)
>        (expr_list:REG_DEAD (reg/v/f:SI 99 [ a ])
>            (nil))))
>
>  from sched1 - dependecies list:
>   --- Region Dependences --- b 3 bb 0
> ;;      insn  code    bb   dep  prio  cost   reservation
> ;;      ----  ----    --   ---  ----  ----   -----------
> ;;       17   115     3     0     4     1   (2_slots+lsu)       : 58 20 18
> ;;       18   116     3     1     3     1   (2_slots+lsu)       : 58 19
> ;;       19   115     3     1     2     1   (2_slots+lsu)       : 58 20
> ;;       20   116     3     2     1     1   (2_slots+lsu)       : 58
> ;;       58   101     3     4     1     1   (3_slots+pcu)       :
>
> As you can see, insn 19 is dependant on 18.
> I think that this should not happen since the arrays has __restrict__.
> It might have something to do with aliasing.
>
> Are you aware of any kind of problems regarding aliasing and vectors?
> Can you think of anything else that might be wrong in my code that
> leads gcc to think that there is a dependency between insn 18 and 19?

It completely depends on the GCC version you are using.  The
MEM_EXPRs suggest that it is maybe 4.4 or 4.5 based?  Because
they are not existant.  Try recent trunk which should work fine.

Richard.

> Thanks, Roy.
>

Re: vectorization, scheduling and aliasing

[roy rosen]

Hi Richard,

2010/4/14, Richard Guenther <richard.guenther@gmail.com>:
> On Wed, Apr 14, 2010 at 8:48 AM, roy rosen <roy.1rosen@gmail.com> wrote:
> > Hi All,
> >
> > I have implemented some vectorization features in my gcc port.
> >
> > In the generated code for this function I can see a scheduling problem:
> >
> > int xxx(int* __restrict__ a, int* __restrict__ b)
> > {
> >    int __restrict__ i;
> >    for (i = 0; i < 8; i++)
> >    {
> >        a[i] = b[i];
> >    }
> >    return 0;
> > }
> >
> > from asmcons:
> >
> > (insn 17 14 18 3 a.c:15 (set (reg:V4HI 105)
> >        (mem:V4HI (reg/v/f:SI 100 [ b ]) [2 S8 A64])) 115 {*movv4hi_load} (nil))
> >
> > (insn 18 17 19 3 a.c:15 (set (mem:V4HI (reg/v/f:SI 99 [ a ]) [2 S8 A64])
> >        (reg:V4HI 105)) 116 {*movv4hi_store} (expr_list:REG_DEAD (reg:V4HI 105)
> >        (nil)))
> >
> > (insn 19 18 20 3 a.c:15 (set (reg:V4HI 106)
> >        (mem:V4HI (plus:SI (reg/v/f:SI 100 [ b ])
> >                (const_int 8 [0x8])) [2 S8 A64])) 115 {*movv4hi_load}
> > (expr_list:REG_DEAD (reg/v/f:SI 100 [ b ])
> >        (nil)))
> >
> > (insn 20 19 58 3 a.c:15 (set (mem:V4HI (plus:SI (reg/v/f:SI 99 [ a ])
> >                (const_int 8 [0x8])) [2 S8 A64])
> >        (reg:V4HI 106)) 116 {*movv4hi_store} (expr_list:REG_DEAD (reg:V4HI 106)
> >        (expr_list:REG_DEAD (reg/v/f:SI 99 [ a ])
> >            (nil))))
> >
> >  from sched1 - dependecies list:
> >   --- Region Dependences --- b 3 bb 0
> > ;;      insn  code    bb   dep  prio  cost   reservation
> > ;;      ----  ----    --   ---  ----  ----   -----------
> > ;;       17   115     3     0     4     1   (2_slots+lsu)       : 58 20 18
> > ;;       18   116     3     1     3     1   (2_slots+lsu)       : 58 19
> > ;;       19   115     3     1     2     1   (2_slots+lsu)       : 58 20
> > ;;       20   116     3     2     1     1   (2_slots+lsu)       : 58
> > ;;       58   101     3     4     1     1   (3_slots+pcu)       :
> >
> > As you can see, insn 19 is dependant on 18.
> > I think that this should not happen since the arrays has __restrict__.
> > It might have something to do with aliasing.
> >
> > Are you aware of any kind of problems regarding aliasing and vectors?
> > Can you think of anything else that might be wrong in my code that
> > leads gcc to think that there is a dependency between insn 18 and 19?
>
> It completely depends on the GCC version you are using.  The
> MEM_EXPRs suggest that it is maybe 4.4 or 4.5 based?  Because
> they are not existant.  Try recent trunk which should work fine.
>
> Richard.
>
> > Thanks, Roy.
> >
>

I have taken 4.5.0 and I still get the same problem.
Do you have any ideas? Shouldn't the restrict tell the compiler that
there is no dependency between these insns?

Thanks, Roy.

Re: vectorization, scheduling and aliasing

[Richard Guenther]

On Thu, Apr 22, 2010 at 6:04 PM, roy rosen <roy.1rosen@gmail.com> wrote:
> Hi Richard,
>
> 2010/4/14, Richard Guenther <richard.guenther@gmail.com>:
>> On Wed, Apr 14, 2010 at 8:48 AM, roy rosen <roy.1rosen@gmail.com> wrote:
>> > Hi All,
>> >
>> > I have implemented some vectorization features in my gcc port.
>> >
>> > In the generated code for this function I can see a scheduling problem:
>> >
>> > int xxx(int* __restrict__ a, int* __restrict__ b)
>> > {
>> >    int __restrict__ i;
>> >    for (i = 0; i < 8; i++)
>> >    {
>> >        a[i] = b[i];
>> >    }
>> >    return 0;
>> > }
>> >
>> > from asmcons:
>> >
>> > (insn 17 14 18 3 a.c:15 (set (reg:V4HI 105)
>> >        (mem:V4HI (reg/v/f:SI 100 [ b ]) [2 S8 A64])) 115 {*movv4hi_load} (nil))
>> >
>> > (insn 18 17 19 3 a.c:15 (set (mem:V4HI (reg/v/f:SI 99 [ a ]) [2 S8 A64])
>> >        (reg:V4HI 105)) 116 {*movv4hi_store} (expr_list:REG_DEAD (reg:V4HI 105)
>> >        (nil)))
>> >
>> > (insn 19 18 20 3 a.c:15 (set (reg:V4HI 106)
>> >        (mem:V4HI (plus:SI (reg/v/f:SI 100 [ b ])
>> >                (const_int 8 [0x8])) [2 S8 A64])) 115 {*movv4hi_load}
>> > (expr_list:REG_DEAD (reg/v/f:SI 100 [ b ])
>> >        (nil)))
>> >
>> > (insn 20 19 58 3 a.c:15 (set (mem:V4HI (plus:SI (reg/v/f:SI 99 [ a ])
>> >                (const_int 8 [0x8])) [2 S8 A64])
>> >        (reg:V4HI 106)) 116 {*movv4hi_store} (expr_list:REG_DEAD (reg:V4HI 106)
>> >        (expr_list:REG_DEAD (reg/v/f:SI 99 [ a ])
>> >            (nil))))
>> >
>> >  from sched1 - dependecies list:
>> >   --- Region Dependences --- b 3 bb 0
>> > ;;      insn  code    bb   dep  prio  cost   reservation
>> > ;;      ----  ----    --   ---  ----  ----   -----------
>> > ;;       17   115     3     0     4     1   (2_slots+lsu)       : 58 20 18
>> > ;;       18   116     3     1     3     1   (2_slots+lsu)       : 58 19
>> > ;;       19   115     3     1     2     1   (2_slots+lsu)       : 58 20
>> > ;;       20   116     3     2     1     1   (2_slots+lsu)       : 58
>> > ;;       58   101     3     4     1     1   (3_slots+pcu)       :
>> >
>> > As you can see, insn 19 is dependant on 18.
>> > I think that this should not happen since the arrays has __restrict__.
>> > It might have something to do with aliasing.
>> >
>> > Are you aware of any kind of problems regarding aliasing and vectors?
>> > Can you think of anything else that might be wrong in my code that
>> > leads gcc to think that there is a dependency between insn 18 and 19?
>>
>> It completely depends on the GCC version you are using.  The
>> MEM_EXPRs suggest that it is maybe 4.4 or 4.5 based?  Because
>> they are not existant.  Try recent trunk which should work fine.
>>
>> Richard.
>>
>> > Thanks, Roy.
>> >
>>
>
> I have taken 4.5.0 and I still get the same problem.
> Do you have any ideas? Shouldn't the restrict tell the compiler that
> there is no dependency between these insns?

It doesn't work for 4.5.0 due to PR42617.

Richard.

> Thanks, Roy.
>

Re: vectorization, scheduling and aliasing

[roy rosen]

Hi Richard,

2010/4/23, Richard Guenther <richard.guenther@gmail.com>:
> On Thu, Apr 22, 2010 at 6:04 PM, roy rosen <roy.1rosen@gmail.com> wrote:
> > Hi Richard,
> >
> > 2010/4/14, Richard Guenther <richard.guenther@gmail.com>:
> >> On Wed, Apr 14, 2010 at 8:48 AM, roy rosen <roy.1rosen@gmail.com> wrote:
> >> > Hi All,
> >> >
> >> > I have implemented some vectorization features in my gcc port.
> >> >
> >> > In the generated code for this function I can see a scheduling problem:
> >> >
> >> > int xxx(int* __restrict__ a, int* __restrict__ b)
> >> > {
> >> >    int __restrict__ i;
> >> >    for (i = 0; i < 8; i++)
> >> >    {
> >> >        a[i] = b[i];
> >> >    }
> >> >    return 0;
> >> > }
> >> >
> >> > from asmcons:
> >> >
> >> > (insn 17 14 18 3 a.c:15 (set (reg:V4HI 105)
> >> >        (mem:V4HI (reg/v/f:SI 100 [ b ]) [2 S8 A64])) 115 {*movv4hi_load} (nil))
> >> >
> >> > (insn 18 17 19 3 a.c:15 (set (mem:V4HI (reg/v/f:SI 99 [ a ]) [2 S8 A64])
> >> >        (reg:V4HI 105)) 116 {*movv4hi_store} (expr_list:REG_DEAD (reg:V4HI 105)
> >> >        (nil)))
> >> >
> >> > (insn 19 18 20 3 a.c:15 (set (reg:V4HI 106)
> >> >        (mem:V4HI (plus:SI (reg/v/f:SI 100 [ b ])
> >> >                (const_int 8 [0x8])) [2 S8 A64])) 115 {*movv4hi_load}
> >> > (expr_list:REG_DEAD (reg/v/f:SI 100 [ b ])
> >> >        (nil)))
> >> >
> >> > (insn 20 19 58 3 a.c:15 (set (mem:V4HI (plus:SI (reg/v/f:SI 99 [ a ])
> >> >                (const_int 8 [0x8])) [2 S8 A64])
> >> >        (reg:V4HI 106)) 116 {*movv4hi_store} (expr_list:REG_DEAD (reg:V4HI 106)
> >> >        (expr_list:REG_DEAD (reg/v/f:SI 99 [ a ])
> >> >            (nil))))
> >> >
> >> >  from sched1 - dependecies list:
> >> >   --- Region Dependences --- b 3 bb 0
> >> > ;;      insn  code    bb   dep  prio  cost   reservation
> >> > ;;      ----  ----    --   ---  ----  ----   -----------
> >> > ;;       17   115     3     0     4     1   (2_slots+lsu)       : 58 20 18
> >> > ;;       18   116     3     1     3     1   (2_slots+lsu)       : 58 19
> >> > ;;       19   115     3     1     2     1   (2_slots+lsu)       : 58 20
> >> > ;;       20   116     3     2     1     1   (2_slots+lsu)       : 58
> >> > ;;       58   101     3     4     1     1   (3_slots+pcu)       :
> >> >
> >> > As you can see, insn 19 is dependant on 18.
> >> > I think that this should not happen since the arrays has __restrict__.
> >> > It might have something to do with aliasing.
> >> >
> >> > Are you aware of any kind of problems regarding aliasing and vectors?
> >> > Can you think of anything else that might be wrong in my code that
> >> > leads gcc to think that there is a dependency between insn 18 and 19?
> >>
> >> It completely depends on the GCC version you are using.  The
> >> MEM_EXPRs suggest that it is maybe 4.4 or 4.5 based?  Because
> >> they are not existant.  Try recent trunk which should work fine.
> >>
> >> Richard.
> >>
> >> > Thanks, Roy.
> >> >
> >>
> >
> > I have taken 4.5.0 and I still get the same problem.
> > Do you have any ideas? Shouldn't the restrict tell the compiler that
> > there is no dependency between these insns?
>
> It doesn't work for 4.5.0 due to PR42617.
>
> Richard.
>
> > Thanks, Roy.
> >
>

I took snapshot 4.6-20100424 (is this version ok?) and the problem
still occurs when I am looking at the vectorized version of the code.
It seems ok in the nonvectorized version.

What might be the problem?

Thanks, Roy.

Re: vectorization, scheduling and aliasing

[Richard Guenther]

On Mon, Apr 26, 2010 at 9:43 AM, roy rosen <roy.1rosen@gmail.com> wrote:
> Hi Richard,
>
> 2010/4/23, Richard Guenther <richard.guenther@gmail.com>:
>> On Thu, Apr 22, 2010 at 6:04 PM, roy rosen <roy.1rosen@gmail.com> wrote:
>> > Hi Richard,
>> >
>> > 2010/4/14, Richard Guenther <richard.guenther@gmail.com>:
>> >> On Wed, Apr 14, 2010 at 8:48 AM, roy rosen <roy.1rosen@gmail.com> wrote:
>> >> > Hi All,
>> >> >
>> >> > I have implemented some vectorization features in my gcc port.
>> >> >
>> >> > In the generated code for this function I can see a scheduling problem:
>> >> >
>> >> > int xxx(int* __restrict__ a, int* __restrict__ b)
>> >> > {
>> >> >    int __restrict__ i;
>> >> >    for (i = 0; i < 8; i++)
>> >> >    {
>> >> >        a[i] = b[i];
>> >> >    }
>> >> >    return 0;
>> >> > }
>> >> >
>> >> > from asmcons:
>> >> >
>> >> > (insn 17 14 18 3 a.c:15 (set (reg:V4HI 105)
>> >> >        (mem:V4HI (reg/v/f:SI 100 [ b ]) [2 S8 A64])) 115 {*movv4hi_load} (nil))
>> >> >
>> >> > (insn 18 17 19 3 a.c:15 (set (mem:V4HI (reg/v/f:SI 99 [ a ]) [2 S8 A64])
>> >> >        (reg:V4HI 105)) 116 {*movv4hi_store} (expr_list:REG_DEAD (reg:V4HI 105)
>> >> >        (nil)))
>> >> >
>> >> > (insn 19 18 20 3 a.c:15 (set (reg:V4HI 106)
>> >> >        (mem:V4HI (plus:SI (reg/v/f:SI 100 [ b ])
>> >> >                (const_int 8 [0x8])) [2 S8 A64])) 115 {*movv4hi_load}
>> >> > (expr_list:REG_DEAD (reg/v/f:SI 100 [ b ])
>> >> >        (nil)))
>> >> >
>> >> > (insn 20 19 58 3 a.c:15 (set (mem:V4HI (plus:SI (reg/v/f:SI 99 [ a ])
>> >> >                (const_int 8 [0x8])) [2 S8 A64])
>> >> >        (reg:V4HI 106)) 116 {*movv4hi_store} (expr_list:REG_DEAD (reg:V4HI 106)
>> >> >        (expr_list:REG_DEAD (reg/v/f:SI 99 [ a ])
>> >> >            (nil))))
>> >> >
>> >> >  from sched1 - dependecies list:
>> >> >   --- Region Dependences --- b 3 bb 0
>> >> > ;;      insn  code    bb   dep  prio  cost   reservation
>> >> > ;;      ----  ----    --   ---  ----  ----   -----------
>> >> > ;;       17   115     3     0     4     1   (2_slots+lsu)       : 58 20 18
>> >> > ;;       18   116     3     1     3     1   (2_slots+lsu)       : 58 19
>> >> > ;;       19   115     3     1     2     1   (2_slots+lsu)       : 58 20
>> >> > ;;       20   116     3     2     1     1   (2_slots+lsu)       : 58
>> >> > ;;       58   101     3     4     1     1   (3_slots+pcu)       :
>> >> >
>> >> > As you can see, insn 19 is dependant on 18.
>> >> > I think that this should not happen since the arrays has __restrict__.
>> >> > It might have something to do with aliasing.
>> >> >
>> >> > Are you aware of any kind of problems regarding aliasing and vectors?
>> >> > Can you think of anything else that might be wrong in my code that
>> >> > leads gcc to think that there is a dependency between insn 18 and 19?
>> >>
>> >> It completely depends on the GCC version you are using.  The
>> >> MEM_EXPRs suggest that it is maybe 4.4 or 4.5 based?  Because
>> >> they are not existant.  Try recent trunk which should work fine.
>> >>
>> >> Richard.
>> >>
>> >> > Thanks, Roy.
>> >> >
>> >>
>> >
>> > I have taken 4.5.0 and I still get the same problem.
>> > Do you have any ideas? Shouldn't the restrict tell the compiler that
>> > there is no dependency between these insns?
>>
>> It doesn't work for 4.5.0 due to PR42617.
>>
>> Richard.
>>
>> > Thanks, Roy.
>> >
>>
>
> I took snapshot 4.6-20100424 (is this version ok?) and the problem
> still occurs when I am looking at the vectorized version of the code.
> It seems ok in the nonvectorized version.
>
> What might be the problem?

Look at the -fdump-tree-optimized-alias dump, likely the defs
of the pointer SSA names that are used in the loads/stores
have their alias information dropped.  Or the vectorizer emits
ALIGN_INDIRECT_REFs on your target which we still drop.

Richard.

> Thanks, Roy.
>

Re: vectorization, scheduling and aliasing

[roy rosen]

Hi Richard,

Here is the relevant block from the dump:

<bb 3>:
  __vect_var__26_6 = *__vect_p_14_19;
  *__vect_p_18_25 = __vect_var__26_6;
  # PT = nonlocal { __PARM_RESTRICT_2 } (restr)
  __vect_p_22_11 = __vect_p_14_19 + 8;
  # PT = nonlocal { __PARM_RESTRICT_1 } (restr)
  __vect_p_27_12 = __vect_p_18_25 + 8;
  __vect_var__26_45 = *__vect_p_22_11;
  *__vect_p_27_12 = __vect_var__26_45;

I guess that it recognizes the restrict pointer so I don't understand
why later it creates a dependency between the insns.
Do you see here something relevant?

Thanks, Roy.

Re: vectorization, scheduling and aliasing

[Richard Guenther]

On Mon, Apr 26, 2010 at 3:42 PM, roy rosen <roy.1rosen@gmail.com> wrote:
> Hi Richard,
>
> Here is the relevant block from the dump:
>
> <bb 3>:
>  __vect_var__26_6 = *__vect_p_14_19;
>  *__vect_p_18_25 = __vect_var__26_6;
>  # PT = nonlocal { __PARM_RESTRICT_2 } (restr)
>  __vect_p_22_11 = __vect_p_14_19 + 8;
>  # PT = nonlocal { __PARM_RESTRICT_1 } (restr)
>  __vect_p_27_12 = __vect_p_18_25 + 8;
>  __vect_var__26_45 = *__vect_p_22_11;
>  *__vect_p_27_12 = __vect_var__26_45;
>
> I guess that it recognizes the restrict pointer so I don't understand
> why later it creates a dependency between the insns.
> Do you see here something relevant?

No, it looks good.  At least if both __vect_p_22 and __vect_p_27
are restrict qualified.

The RTL dumps should show *__vect_p_27_12 and similar
expressions in their MEM_ATTRS as well.

Note that IVOPTs is known to not preserve alias information so
you might want to try -fno-ivopts

> Thanks, Roy.
>

Re: vectorization, scheduling and aliasing

[roy rosen]

Hi,

I have looked a bit more and tried also ia-64 and bfin and actually I
can't find a single example where vectorized code using __restrict__
variables would break the dependency between stores and loads.

for this simple program:

unsigned short xxx(unsigned short* __restrict__ a, unsigned short*
__restrict__ b)
{
   int i;
   for (i = 0; i < 8; i++)
   {
       a[i] = b[i];
   }
   return 0;
}

After unrolling and vectorization I get for all architectures that
there is a dependency between the first store (to a) and the second
load (from b).
since these are restrict variables I think this behavior is incorrect.

Can someone please show me such an example? Or is this feature not yet
supported?

Thanks, Roy.