[Polly][PATCH 4/8] Allow the IslExprBuilder to compare pointers

[Johannes Doerfert]

On 08/11, Tobias Grosser wrote:
> On 11/08/2014 10:56, Johannes Doerfert wrote:
> >On 08/11, Tobias Grosser wrote:
> >>On 11/08/2014 09:38, Johannes Doerfert wrote:
> >>>On 08/11, Tobias Grosser wrote:
> >>>>Please merge this into the patch that actually tests and uses it.
> >>>>
> >>>>Some comments inline:
> >>>>
> >>>>On 10/08/2014 09:50, Johannes Doerfert wrote:
> >>>>[..]
> >>>>>    switch (isl_ast_expr_get_op_type(Expr)) {
> >>>>>    default:
> >>>>>@@ -280,16 +286,20 @@ Value *IslExprBuilder::createOpICmp(__isl_take isl_ast_expr *Expr) {
> >>>>>      Res = Builder.CreateICmpEQ(LHS, RHS);
> >>>>>      break;
> >>>>>    case isl_ast_op_le:
> >>>>>-    Res = Builder.CreateICmpSLE(LHS, RHS);
> >>>>>+    Res = IsPtrType ? Builder.CreateICmpULE(LHS, RHS)
> >>>>>+                    : Builder.CreateICmpSLE(LHS, RHS);
> >>>>>      break;
> >>>>>    case isl_ast_op_lt:
> >>>>>-    Res = Builder.CreateICmpSLT(LHS, RHS);
> >>>>>+    Res = IsPtrType ? Builder.CreateICmpULT(LHS, RHS)
> >>>>>+                    : Builder.CreateICmpSLT(LHS, RHS);
> >>>>>      break;
> >>>>>    case isl_ast_op_ge:
> >>>>>-    Res = Builder.CreateICmpSGE(LHS, RHS);
> >>>>>+    Res = IsPtrType ? Builder.CreateICmpUGE(LHS, RHS)
> >>>>>+                    : Builder.CreateICmpSGE(LHS, RHS);
> >>>>>      break;
> >>>>>    case isl_ast_op_gt:
> >>>>>-    Res = Builder.CreateICmpSGT(LHS, RHS);
> >>>>>+    Res = IsPtrType ? Builder.CreateICmpUGT(LHS, RHS)
> >>>>>+                    : Builder.CreateICmpSGT(LHS, RHS);
> >>>>
> >>>>Did you consider CreateICmp(Predicate, LHS, RHS), plus a table that
> >>>>maps the enum values to the corresponding predicates? This code seems
> >>>>to start to have a lot of duplication.
> >>>I did not really, but we could use a two dimensional table if you prefer that.
> >>
> >>Does a 2D table work? I think we would need a map for this, as the index is
> >>an enum, no?
> >Yes, isl enums = C enums = int
> >
> >>Otherwise, we could just make this two switch statements, that select
> >>the ICmp predicate and than just have a single call to Builder.CreateImp at
> >>the end. What do you think?
> >I don't have any strong feeling about it, or in other words I think
> >that's a lot of discussion about 10 lines of code which just builds a
> >compare instruction.
> >
> >Tell me what you want and I change it to whatever.
> 
> I would pull out the Builder.Create* from the switch statement and instead
> just select the predicate.
> 
> We can then use Builder.CreateICmp(Predicate, LHS, RHS) after the switch.
That answers only a quarter of the questions. You mentioned a table
(which I interpreted as a 2D table), a map and "two switch statements"
in order to select the right Predicate (which is the actuall challenge here).

What is it going to be?

> 
> Cheers,
> Tobias

-- 

Johannes Doerfert
Researcher / PhD Student

Compiler Design Lab (Prof. Hack)
Saarland University, Computer Science
Building E1.3, Room 4.26

Tel. +49 (0)681 302-57521 : doerfert at cs.uni-saarland.de
Fax. +49 (0)681 302-3065  : http://www.cdl.uni-saarland.de/people/doerfert
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 213 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140811/da90a894/attachment.sig>