[PATCH] D87188: [InstCombine] Canonicalize SPF to abs intrinc

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

In D87188#2463447 <https://reviews.llvm.org/D87188#2463447>, @dmgreen wrote:

> Yeah. The reproducer seems to work OK with a patch something like this:
>
>   diff --git a/llvm/lib/Analysis/InstructionSimplify.cpp b/llvm/lib/Analysis/InstructionSimplify.cpp
>   index 35c21a0..c517286 100644                                                                     
>   --- a/llvm/lib/Analysis/InstructionSimplify.cpp                                                   
>   +++ b/llvm/lib/Analysis/InstructionSimplify.cpp                                                   
>   @@ -4020,11 +4020,11 @@ static Value *simplifySelectWithICmpCond(Value *CondVal, Value *TrueVal,  
>                                                                                                     
>        // X == 0 ? abs(X) : -abs(X) --> -abs(X)                                                     
>        // X == 0 ? -abs(X) : abs(X) --> abs(X)                                                      
>   -    if (match(TrueVal, m_Intrinsic<Intrinsic::abs>(m_Value(X))) &&                               
>   -        match(FalseVal, m_Neg(m_Intrinsic<Intrinsic::abs>(m_Specific(X)))))                      
>   +    if (match(TrueVal, m_Intrinsic<Intrinsic::abs>(m_Specific(CmpLHS))) &&                       
>   +        match(FalseVal, m_Neg(m_Intrinsic<Intrinsic::abs>(m_Specific(CmpLHS)))))                 
>          return FalseVal;                                                                           
>   -    if (match(TrueVal, m_Neg(m_Intrinsic<Intrinsic::abs>(m_Value(X)))) &&                        
>   -        match(FalseVal, m_Intrinsic<Intrinsic::abs>(m_Specific(X))))                             
>   +    if (match(TrueVal, m_Neg(m_Intrinsic<Intrinsic::abs>(m_Specific(CmpLHS)))) &&                
>   +        match(FalseVal, m_Intrinsic<Intrinsic::abs>(m_Specific(CmpLHS))))                        
>          return FalseVal;                                                                           
>      }                                                                                              
>                                                                                                     

Indeed, that is what i have already committed locally and will push in a sec.

> (I must admit, was fully expecting this to be something wrong in the AArch64 backend. I'll leave that fix to you if you are happy to add tests and whatnot.)

Yeah, i also was almost convinced it was :)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D87188/new/

https://reviews.llvm.org/D87188