[llvm-commits] SETCC Patches (For Review Only) #1: llvm-gcc

[Chris Lattner]

>>
>> +  case LT_EXPR: {
>> +    tree Op0Ty = TREE_TYPE(TREE_OPERAND(exp,0));
>> +    if (!FLOAT_TYPE_P(Op0Ty))
>> +      Result =
>> +        EmitCompare(exp, TYPE_UNSIGNED(Op0Ty) ?
>> +                    ICmpInst::ICMP_ULT : ICmpInst::ICMP_SLT);
>> +    else
>> +      Result = EmitCompare(exp, Instruction::SetLT, 0);
>> +    break;
>> +  }
>> +  case LE_EXPR: {
>> +    tree Op0Ty = TREE_TYPE(TREE_OPERAND(exp,0));
>> +    if (!FLOAT_TYPE_P(Op0Ty))
>> +      Result =
>> +        EmitCompare(exp, TYPE_UNSIGNED(Op0Ty) ?
>> +                    ICmpInst::ICMP_ULE : ICmpInst::ICMP_SLE);
>> +    else
>> +      Result = EmitCompare(exp, Instruction::SetLE, 0);
>> +    break;
>> +  }
>>
>>
>> This logic shouldn't be duplicated everywhere.  I much prefer that  
>> you
>> do something like:
>>
>>
>> case LT_EXPR:
>>   Result = EmitCompare(exp, ICmpInst::ICMP_ULT,
>> ICmpInst::ICMP_SLT, Instruction::SetLT, 0);
>>   break;
>
> That's really gross. One or two arguments will always be ignored
> depending on whether it is a floating point or an integer compare. If
> your objection is the overloading, how about EmitFPCompare and
> EmitIntCompare?  If you want to shorten the logic in the switch  
> cases we
> could write an inline name EmitCompare to do what you suggested.

I don't understand, why is that gross?  Basically EmitCompare will  
have this at the top:

Value *EmitCompare(tree exp, unsigned UIOpc, SIOpc, FPOpc, ...) {
   tree Op0Ty = TREE_TYPE(TREE_OPERAND(exp,0));
   unsigned Predicate = FPOpc;
   if (!FLOAT_TYPE_P(Op0Ty))
     Predicate = TYPE_UNSIGNED(Op0Ty) ? UIOpc : SIOpc;


This doesn't seem gross.

-Chris