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

Tobias Grosser tobias at grosser.es
Mon Aug 11 00:46:46 PDT 2014


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?

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?

Cheers,
Tobias



More information about the llvm-commits mailing list