On 07/28/2011 06:56 PM, Sebastian Pop wrote:
Hi Tobi,

On Thu, Jul 28, 2011 at 12:13, Tobias Grosser<tob...@grosser.es>  wrote:
+  struct clast_user_stmt *body
+    = clast_get_body_of_loop ((struct clast_stmt *) stmt);

I am not a big fan of using clast_get_body_of_loop as it is buggy.
Introducing new uses of it, is nothing what I would support. Do we really
need this?

No, because of ...


+  poly_bb_p pbb = (poly_bb_p) cloog_statement_usr (body->statement);

What about some more meaningful names like bound_one, bound_two?

Ok, see the second patch attached.

+
+  compute_bounds_for_level (pbb, level, v1, v2);

Mh. I do not completely understand all the code. But can't we get v1 and v2
set without the need for the compute_bounds_for_level function. Is the
type_for_clast_expression not setting them.


... this.
You are right.  type_for_clast_expr would provide the bounds for the
RHS of the assign and so we don't need to compute the bounds on
the loop level, as we would have done on a real loop.  Attached the
amended patch.  I'm regstrapping these patches on amd64-linux.
Ok for trunk after?

Looks good to me. Please commit if it passes regstrapping.

Cheers
Tobi

Reply via email to