On 2021-07-14 04:50, Segher Boessenkool wrote:
Hi!
On Tue, Jul 13, 2021 at 08:50:46PM +0800, Jiufu Guo wrote:
* doc/tm.texi: Regenerated.
Pet peeve: "Regenerate.", no "d".
Ok, yeap. While, 'Regenerate and Regenerated' were used by commits
somewhere :)
+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.
Ok, thanks.
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 :-)
Yes, the preferred 'doloop iv mode' from targets should be a MODE_INT.
I will add comments, and update the gcc_assert you mentioned below
for this.
Thanks a lot for your comments and suggestions!
When writing words, I was always from adding/deleting and still hard to
get
perfect ones -:(
+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.
Thanks! I will update accordingly. And I'm thinking to add tests to
check
doloop.xx type: no zero_extend to access subreg. This is the intention
of this patch.
+/* 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.
Ok, will separate as two asserts.
+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.
The generic code checks if the returned mode can works on doloop iv
correctly,
if the preferred mode is not suitable (e.g. preferred_doloop_mode
returns DI,
but niter is a large value in TI), then doloop.xx IV will use the
original mode.
When a target really prefer TImode, and TImode can represent number of
iterations,
This would still work. In current code, word_mode is SI/DI in most
targets, like Pmode.
On powerpc, they are DImode (for 64bit)/SImode(32bit)
Thanks again for your comments!
BR,
Jiufu
Segher
