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