[Openmp-commits] [PATCH] D125717: [InstCombine] Optimize and of icmps with power-of-2 and contiguous masks

Noah Goldstein via Phabricator via Openmp-commits openmp-commits at lists.llvm.org
Wed May 17 16:09:37 PDT 2023


goldstein.w.n added inline comments.


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp:543
+    // Test scalar type arguments for conversion.
+    if (!isReducible(B, D, E))
+      return nullptr;
----------------
nit: maybe:
```
} 
// Test scalar type arguments for conversion.    
else if (!isReducible(B, D, E)) {
     return nullptr;
}
```

Although given the LLVM style of '}' and 'else' on the same line its a bit out of place. No strong feelings so update if you see fit.


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp:610
 /// Try to fold (icmp(A & B) ==/!= C) &/| (icmp(A & D) ==/!= E)
-/// into a single (icmp(A & X) ==/!= Y).
+/// into a single (icmp(A & X) ==/!= Y) or (icmp A u< Y).
 static Value *foldLogOpOfMaskedICmps(ICmpInst *LHS, ICmpInst *RHS, bool IsAnd,
----------------
When do we transform to `A & X eq/ne Y`? I don't see that code.


================
Comment at: llvm/test/Transforms/InstCombine/icmp-logical.ll:2553
   ret <6 x i1> %t4
 }
----------------
One more thing. Can you put the "fail" keyword somewhere in the tests that you expect no change in. Think it helps make it crystal clear exact what the is meant to happen.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D125717



More information about the Openmp-commits mailing list