[PATCH] D49238: [InstCombine] add more SPFofSPF folding

[ChenZheng via Phabricator via llvm-commits]

shchenz marked 2 inline comments as done.
shchenz added inline comments.


================
Comment at: llvm/lib/Analysis/ValueTracking.cpp:4640-4641
     // Set LHS and RHS so that RHS is the negated operand of the select
+    bool CmpUsesNegatedOp = match(CmpLHS, m_Neg(m_Specific(TrueVal))) ||
+                        match(CmpLHS, m_Neg(m_Specific(FalseVal)));
     if (match(TrueVal, MaybeSExtLHS)) {
----------------
spatel wrote:
> What happens if the cmp uses a negated and sign-extended op? I'm not sure if this is possible because of other transforms. Do we have test(s) that include that possibility?
new patch can recognize:
1: 
%ext = sext i16 %tmp to i32
%b = sub i32 0, %ext
%cond = icmp sgt i32 %b, -1
%ret = select i1 %cond %b, %ext

%ext can be any operator's result including sext. So I 

2: 
%ext = sext i16 %tmp to i32
%b = sub i32 0, %ext
%cond = icmp sgt i32 %ext, -1
%ret = select i1 %cond %b, %ext

in this case, it is the same logic with trunk, so I think there should be already test for it. (abs_canonical_5/nabs_canonical_5 in abs-1.ll)

3: 
%b = sub i16 0, %tmp
%ext = sext i16 %b to i32
%cond = icmp sgt i32 %ext, -1
%ret = select i1 %cond %tmp, %X

for this case:
if %X is %b, in trunk and new patch, this is not a abs pattern.
if %X is %ext, in trunk and new patch, this is also not a abs pattern.

@spatel  Sanjay, Do you have any other pattern I have not considered about? 

But there should be some thing we can do for sign-extended to enlarge abs pattern scope. for example We should recognize (%tmp, %ext) as negation pair in isKnownNegation(). Maybe I will do this after I finish current work for abs.


================
Comment at: llvm/test/Transforms/InstCombine/abs_abs.ll:333-336
 ; CHECK-LABEL: @abs_abs_x02_vec(
 ; CHECK-NEXT:    [[SUB:%.*]] = sub nsw <2 x i32> zeroinitializer, [[X:%.*]]
 ; CHECK-NEXT:    [[CMP:%.*]] = icmp sgt <2 x i32> [[SUB]], <i32 -1, i32 -1>
 ; CHECK-NEXT:    [[COND:%.*]] = select <2 x i1> [[CMP]], <2 x i32> [[SUB]], <2 x i32> [[X]]
----------------
spatel wrote:
> Do you know why the scalar version above folded, but the vector version did not? Something is falsely excluding vector types.
Yes, it is also confuse me. I will look into it. Maybe there is a bug in trunk.


https://reviews.llvm.org/D49238