[PATCH] D94089: [Reassociate] move check to ignore boolean expressions before canonicalizing binary operands

[Marius Hillenbrand via Phabricator via llvm-commits]

mhillenbrand updated this revision to Diff 319550.
mhillenbrand added a comment.

Add regression test


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94089

Files:
  llvm/lib/Transforms/Scalar/Reassociate.cpp
  llvm/test/Transforms/Reassociate/pr48529.ll


Index: llvm/test/Transforms/Reassociate/pr48529.ll
===================================================================
--- /dev/null
+++ llvm/test/Transforms/Reassociate/pr48529.ll
@@ -0,0 +1,34 @@
+; RUN: opt < %s -reassociate -S | FileCheck %s
+; PR48529
+
+; reduced from code with the structure
+;
+; float ab = a * b;
+; float ac = a * c;
+; float bc = b * c;
+; ...
+;
+; if (ab > de || ac > df || bc > cd)
+;     return 0;
+; ...
+; return 1;
+;
+; clang first produces short-circuiting control flow
+; which simplify-cfg contracts to "or i1"
+
+define i32 @simple_reduced() {
+lor.lhs.false:
+  %mul4 = fmul contract float undef, undef
+  %cmp6 = fcmp contract ogt float undef, %mul4
+  %cmp8 = fcmp contract ogt float undef, undef
+  %or.cond2 = or i1 %cmp6, %cmp8
+; CHECK: simple_reduced
+; CHECK: %or.cond2 = or i1 %cmp6, %cmp8
+  br i1 %or.cond2, label %return, label %if.end
+
+if.end:
+  ret i32 1
+
+return:
+  ret i32 0
+}
Index: llvm/lib/Transforms/Scalar/Reassociate.cpp
===================================================================
--- llvm/lib/Transforms/Scalar/Reassociate.cpp
+++ llvm/lib/Transforms/Scalar/Reassociate.cpp
@@ -2187,6 +2187,15 @@
       I = NI;
     }
 
+  // Do not reassociate boolean (i1) expressions.  We want to preserve the
+  // original order of evaluation for short-circuited comparisons that
+  // SimplifyCFG has folded to AND/OR expressions.  If the expression
+  // is not further optimized, it is likely to be transformed back to a
+  // short-circuited form for code gen, and the source order may have been
+  // optimized for the most likely conditions.
+  if (I->getType()->isIntegerTy(1))
+    return;
+
   // Commute binary operators, to canonicalize the order of their operands.
   // This can potentially expose more CSE opportunities, and makes writing other
   // transformations simpler.
@@ -2201,14 +2210,6 @@
   if (I->getType()->isFPOrFPVectorTy() && !I->isFast())
     return;
 
-  // Do not reassociate boolean (i1) expressions.  We want to preserve the
-  // original order of evaluation for short-circuited comparisons that
-  // SimplifyCFG has folded to AND/OR expressions.  If the expression
-  // is not further optimized, it is likely to be transformed back to a
-  // short-circuited form for code gen, and the source order may have been
-  // optimized for the most likely conditions.
-  if (I->getType()->isIntegerTy(1))
-    return;
 
   // If this is a bitwise or instruction of operands
   // with no common bits set, convert it to X+Y.


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D94089.319550.patch
Type: text/x-patch
Size: 2524 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20210127/8dd8044e/attachment.bin>