On 2021-05-01 05:37, Segher Boessenkool wrote:
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."
Thanks for point out this, yes, it may be better to call it as 'wrap' :)
+-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?
Would update it accordingly. Thanks for your suggestion!
@@ -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 ;-) )
Yeap, thanks!
+/* 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.
Thanks :)
+ /* 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"?
Thanks, the comment should be more clear, the intention is:
make sure "lhs/rhs" pair are "index/bound" pair.
+
+ /* 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?
The intention is "If there is no wrap/overflow happen", no need to split
loop".
If the base is a known value, the index may not wrap/overflow and may be
able
optimized by other passes.
Using range-info to check wrap/overflow could be a 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 is better wording :)
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 :-) )
Thanks so much for your very helpful comments!
Jiufu Guo.
Segher
