[llvm-commits] [PATCH][fast-math, Instcombine] X * (cond ? 1.0 : 0.0) => cond ? X : 0.0

[Shuxin Yang]

Hi, Jakub:

   IMHO,  your code could be little  bit faster by taking a glimpse of 
both operands, and then take a closer look
if the Selects match "cond ? 1/0 : 0/1".

   i.e.
   if (opnd0 is select || opnd1 is select) {
        your original code.
   }

    The rationale is that more often than not, we cannot match this 
case. So for common case, the cost is
only "taking glimpse".

Thanks
Shuxin



On 12/14/12 11:34 AM, Shuxin Yang wrote:
> Hi, Jakub:
>
>    Yes, you are right. My code will miss the case when both operands 
> are Select.
> Please feel free to replace my code with yours (Or, do I need to 
> revert my code first).
>
>     Please note that I only "READ" your code, I didn't "REVIEW" the 
> code --
> I'm not eligible for code review.
>
> Thanks
> Shuxin
>
> On 12/14/12 11:08 AM, Jakub Staszak wrote:
>> The only thing which concerns me is case where both of FMUL operands 
>> are SelectInst:
>>
>> define double @select4(i32 %cond, double %x, double %y) {
>>    %tobool = icmp ne i32 %cond, 0
>>    %tobool2 = icmp ne i32 %cond, 1
>>    %v0 = select i1 %tobool2, double %x, double %y
>>    %cond1 = select i1 %tobool, double 0.000000e+00, double 1.000000e+00
>>    %mul = fmul nnan nsz double %cond1, %v0
>>    %add = fadd double %mul, %y
>>    ret double %add
>> }
>>
>> Your code won't optimized because first "Match" will be successful, 
>> and then it will fail to match ConstantFP.
>>
>> I have an alternative, which might be a little bit more expensive, 
>> but handles the code above.
>> (If we decide to drop "match" and use old-school dyn_cast<> it might 
>> be cheaper though.)
>>
>> +  // X * cond ? 1.0 : 0.0 => cond ? X : 0.0
>> +  if (I.hasNoNaNs() && I.hasNoSignedZeros()) {
>> +    Value *V0 = I.getOperand(0);
>> +    Value *V1 = I.getOperand(1);
>> +    Value *Cond;
>> +
>> +    Value *T  = 0;
>> +    if (match(V0, m_Select(m_Value(Cond), m_Zero(), m_FPOne())))
>> +      // (cond ? 0.0 : 1.0) * V1
>> +      T = Builder->CreateSelect(Cond, 
>> cast<SelectInst>(V0)->getOperand(1), V1);
>> +    else if (match(V0, m_Select(m_Value(Cond), m_FPOne(), m_Zero())))
>> +      // (cond ? 1.0 : 0.0) * V1
>> +      T = Builder->CreateSelect(Cond, 
>> cast<SelectInst>(V0)->getOperand(2), V1);
>> +    else if (match(V1, m_Select(m_Value(Cond), m_Zero(), m_FPOne())))
>> +      // V0 * (cond ? 0.0 : 1.0)
>> +      T = Builder->CreateSelect(Cond, 
>> cast<SelectInst>(V1)->getOperand(1), V0);
>> +    else if (match(V1, m_Select(m_Value(Cond), m_FPOne(), m_Zero())))
>> +      // V0 * (cond ? 1.0 : 0.0)
>> +      T = Builder->CreateSelect(Cond, 
>> cast<SelectInst>(V1)->getOperand(2), V0);
>> +
>> +    if (T)
>> +      return ReplaceInstUsesWith(I, T);
>> +  }
>> +
>>
>>
>> - Kuba
>>
>> On Dec 14, 2012, at 6:58 PM, Nadav Rotem <nrotem at apple.com> wrote:
>>
>>> LGTM.
>>>
>>>
>>> On Dec 13, 2012, at 2:24 PM, Shuxin Yang <shuxin.llvm at gmail.com> wrote:
>>>
>>>> Hi,
>>>>
>>>> The attached patch is to implement rule:
>>>>   X * (cond ? 1.0 : 0.0) => Cond ? X : 0.0
>>>>
>>>> The multiply must be flagged n-signed-zero and n-nan.
>>>>
>>>> Thanks
>>>> Shuxin
>>>> <mpy_select.patch>_______________________________________________
>>>> llvm-commits mailing list
>>>> llvm-commits at cs.uiuc.edu
>>>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>> _______________________________________________
>>> llvm-commits mailing list
>>> llvm-commits at cs.uiuc.edu
>>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>