[PATCH] D17859: [InstCombine] convert 'isPositive' and 'isNegative' vector comparisons to shifts (PR26701, PR26819)

Chandler Carruth via llvm-commits llvm-commits at lists.llvm.org
Thu May 12 19:53:43 PDT 2016


chandlerc accepted this revision.
chandlerc added a comment.
This revision is now accepted and ready to land.

LGTM -- I think this is the best option we have for now.

I think we should add support for the ge/le icmp variants to any transforms that we end up with test cases for where we have a constant at a boundary here. This is no worse than completely removing the canonicalization, but lets us be lazy and allows the easy cases to still collapse to the same predictable structure.


================
Comment at: lib/Transforms/InstCombine/InstCombineCompares.cpp:3145-3146
@@ +3144,4 @@
+    // Unlike the scalar case above, we can't rely on any previous
+    // simplification to handle this for us (although it would make sense for
+    // InstSimplify to handle cases with min/max value *splatted* constants).
+    
----------------
I'd not have the comment talk about a missing instsimplify pass as we'll likely fail to remove it when adding that...


http://reviews.llvm.org/D17859





More information about the llvm-commits mailing list