[patch] Lno branch merge part 8 -- canonical induction variable creation

Roger Sayle roger@eyesopen.com
Sun Aug 22 18:38:00 GMT 2004


On Sun, 22 Aug 2004, Zdenek Dvorak wrote:
> first let me emphasize one thing that you probably missed (as some
> of your comments suggest):  The ivcanon patch does not have problems
> with semantics of overflow of signed arithmetics, since all induction
> variables it creates are unsigned.

My apologies for confusing the issue.

In RTH's review of your patch
http://gcc.gnu.org/ml/gcc-patches/2004-08/msg00364.html

> > +   type = TREE_TYPE (niter);
> > +   niter = fold (build (PLUS_EXPR, type,
> > + 		       niter, convert (type, integer_one_node)));
>
> I don't see anything in canonicalize_loop_induction_variables
> that ensures that this addition doesn't overflow.  If TYPE is
> unsigned I guess it doesn't matter -- with the predecrement
> and EQ_EXPR we can iterate 2**32 times with a 32-bit variable.
> But other parts of the compiler are going to do the wrong thing
> if TYPE is signed.


To which you had originally replied
http://gcc.gnu.org/ml/gcc-patches/2004-08/msg00383.html
> Even in signed type it works exactly the same way.


Which sparked this entire goose chase.  Clearly by now, you understand
that, by default, signed and unsigned overflow don't behave the same way.
Instead of this inflamatory comment, what you really should have said
was:

The code in number_of_iterations_cond in tree-ssa-loop-niter.c that
initially determines the number of iterations, always returns an
unsigned result:
>  /* Count the number of iterations.  */
>  niter_type = unsigned_type_for (type);

And this type is used for creating the canonical induction variables.
Hence at the hunk queried by r~, the type is always guaranteed to be
unsigned, and hence overflow is well defined.


This should be sufficient to answer the reviewer's query and is
independent of whether or not you ever completely understand how
integer overflow works within GCC.


The one comment I will make, now that you've goaded me into reading
through your patch, is that all uses of "build" should really be
using "buildN" and all uses of "convert" should really be using
"fold_convert".

Hopefully, if you resubmit your patch with these changes, and RTH's
suggested tree_int_cst_compare and use of MINUS_EXPR with integer_one_node
instead of PLUS_EXPR with integer_minus_one_node, you should be one step
closer to getting this patch approved.


Roger
--



More information about the Gcc-patches mailing list