Hi!

On Tue, Jul 13, 2021 at 08:50:46PM +0800, Jiufu Guo wrote:
>       * doc/tm.texi: Regenerated.

Pet peeve: "Regenerate.", no "d".

> +DEFHOOK
> +(preferred_doloop_mode,
> + "This hook returns a more preferred mode or the @var{mode} itself.",
> + machine_mode,
> + (machine_mode mode),
> + default_preferred_doloop_mode)

You need a bit more description here.  What does the value it returns
mean?  If you want to say "a more preferred mode or the mode itself",
you should explain what the difference means, too.

You also should say the hook does not need to test if things will fit,
since the generic code already does.

And say this should return a MODE_INT always -- you never test for that
as far as I can see, but you don't need to, as long as everyone does the
sane thing.  So just state every hok implementation should :-)

> +extern machine_mode
> +default_preferred_doloop_mode (machine_mode);

One line please (this is a declaration).

> +/* { dg-do compile } */
> +/* { dg-options "-O2" } */
> +void foo(int *p1, long *p2, int s)
> +{
> +  int n, v, i;
> +
> +  v = 0;
> +  for (n = 0; n <= 100; n++) {
> +     for (i = 0; i < s; i++)
> +        if (p2[i] == n)
> +           p1[i] = v;
> +     v += 88;
> +  }
> +}
> +
> +/* { dg-final { scan-assembler-not {\mrldicl\M} } } */

That is a pretty fragile thing to test for.  It also need a line or two
of comment in the test case what this does, what kind of thing it does
not want to see.

> +/* If PREFERRED_MODE is suitable and profitable, use the preferred
> +   PREFERRED_MODE to compute doloop iv base from niter: base = niter + 1.  */
> +
> +static tree
> +compute_doloop_base_on_mode (machine_mode preferred_mode, tree niter,
> +                          const widest_int &iterations_max)
> +{
> +  tree ntype = TREE_TYPE (niter);
> +  tree pref_type = lang_hooks.types.type_for_mode (preferred_mode, 1);
> +
> +  gcc_assert (pref_type && TYPE_UNSIGNED (ntype));

Should that be pref_type instead of ntype?  If not, write it as two
separate asserts please.

> +static machine_mode
> +rs6000_preferred_doloop_mode (machine_mode)
> +{
> +  return word_mode;
> +}

This is fine if the generic code does the right thing if it passes say
TImode here, and if it never will pass some other mode class mode.


Segher

Reply via email to