Hi!
On Thu, Apr 29, 2021 at 05:50:48PM +0800, Jiufu Guo wrote:
> When there is the possibility that overflow may happen on the loop index,
> a few optimizations would not happen. For example code:
>
> foo (int *a, int *b, unsigned k, unsigned n)
> {
> while (++k != n)
> a[k] = b[k] + 1;
> }
>
> For this code, if "l > n", overflow may happen. if "l < n" at begining,
> it could be optimized (e.g. vectorization).
FWIW, this isn't called "overflow" in C: all overflow is undefined
behaviour.
"A computation involving unsigned operands can never overflow, because a
result that cannot be represented by the resulting unsigned integer type
is reduced modulo the number that is one greater than the largest value
that can be represented by the resulting type."
> +-param=max-insns-ne-cond-split=
> +Common Joined UInteger Var(param_max_insn_ne_cond_split) Init(64) Param
> Optimization
> +The maximum threshold for insnstructions number of a loop with ne condition
> to split.
"number of instructions".
Perhaps you should mark up "ne" as a codeword somehow, but because it
is in a help text it is probably better to just, write out "not equal"
or similar?
> @@ -248,13 +250,14 @@ connect_loop_phis (class loop *loop1, class loop
> *loop2, edge new_e)
> !gsi_end_p (psi_first);
> gsi_next (&psi_first), gsi_next (&psi_second))
> {
> - tree init, next, new_init;
> + tree init, next, new_init, prev;
> use_operand_p op;
> gphi *phi_first = psi_first.phi ();
> gphi *phi_second = psi_second.phi ();
>
> init = PHI_ARG_DEF_FROM_EDGE (phi_first, firste);
> next = PHI_ARG_DEF_FROM_EDGE (phi_first, firstn);
> + prev = PHI_RESULT (phi_first);
> op = PHI_ARG_DEF_PTR_FROM_EDGE (phi_second, seconde);
> gcc_assert (operand_equal_for_phi_arg_p (init, USE_FROM_PTR (op)));
>
I would just declare it at the first use... Less mental load for the
reader. (And a smaller patch ;-) )
> +/* Check if the LOOP exit branch likes "if (idx != bound)".
> + if INV is not NULL and the branch is "if (bound != idx)", set *INV to
> true.
"If INV", sentences start with a capital.
> + /* Make sure idx and bound. */
> + tree idx = gimple_cond_lhs (cond);
> + tree bnd = gimple_cond_rhs (cond);
> + if (expr_invariant_in_loop_p (loop, idx))
> + {
> + std::swap (idx, bnd);
> + if (inv)
> + *inv = true;
> + }
> + else if (!expr_invariant_in_loop_p (loop, bnd))
> + continue;
Make sure idx and bound what? What about them?
> + /* Make sure idx is iv. */
> + class loop *useloop = loop_containing_stmt (cond);
> + affine_iv iv;
> + if (!simple_iv (loop, useloop, idx, &iv, false))
> + continue;
"Make sure idx is a simple_iv"?
> +
> + /* No need to split loop, if base is know value.
> + Or check range info. */
"if base is a known value". Not sure what you mean with range info?
A possible future improvement?
> + /* There is type conversion on idx(or rhs of idx's def).
> + And there is converting shorter to longer type. */
> + tree type = TREE_TYPE (idx);
> + if (!INTEGRAL_TYPE_P (type) || TREE_CODE (idx) != SSA_NAME
> + || !TYPE_UNSIGNED (type)
> + || TYPE_PRECISION (type) == TYPE_PRECISION (sizetype))
> + continue;
"IDX is an unsigned type that is widened to SIZETYPE" etc.
This code assumes SIZETYPE is bigger than any other integer type. Is
that true? Even if so, the second comment could be improved.
(Not reviewing further, my Gimple isn't near good enough, sorry. But
at least to my untrained eye it looks pretty good :-) )
Segher
