[llvm-commits] [llvm] r58946 - in /llvm/trunk: lib/Transforms/Scalar/InstructionCombining.cpp test/FrontendC/2008-11-08-InstCombineSelect.c

Bill Wendling isanbard at gmail.com
Sun Nov 9 15:17:43 PST 2008


Author: void
Date: Sun Nov  9 17:17:42 2008
New Revision: 58946

URL: http://llvm.org/viewvc/llvm-project?rev=58946&view=rev
Log:
The method of doing the matching with a 'select' instruction was wrong. The
original code was matching like this:

	if (match(A, m_Not(m_Value(B))))

B was already matched as a 'select' instruction. However, this isn't matching
what we think it's matching. It would match B as a 'Value', so basically
anything would match to it. In this case, a Constant matched. B was replaced
with a constant representation. And then the wrong value would be used in the
SelectInst::Create statement, causing a crash.

After thinking on this for a moment, and after Nick L. told me how the pattern
matching stuff was supposed to work, the solution was to match NOT an m_Value,
but an m_Select.

Added:
    llvm/trunk/test/FrontendC/2008-11-08-InstCombineSelect.c
Modified:
    llvm/trunk/lib/Transforms/Scalar/InstructionCombining.cpp

Modified: llvm/trunk/lib/Transforms/Scalar/InstructionCombining.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/InstructionCombining.cpp?rev=58946&r1=58945&r2=58946&view=diff

==============================================================================
--- llvm/trunk/lib/Transforms/Scalar/InstructionCombining.cpp (original)
+++ llvm/trunk/lib/Transforms/Scalar/InstructionCombining.cpp Sun Nov  9 17:17:42 2008
@@ -4352,55 +4352,62 @@
       }
     }
 
+#define SELECT_MATCH(Val) \
+    m_Select(m_Value(Val), m_ConstantInt(0), m_ConstantInt(-1))
+#define GET_SELECT_COND(Val) \
+    cast<User>(Val)->getOperand(0)
+
     // (A & (C0?-1:0)) | (B & ~(C0?-1:0)) ->  C0 ? A : B, and commuted variants
     if (match(A, m_Select(m_Value(), m_ConstantInt(-1), m_ConstantInt(0)))) {
-      if (match(D, m_Not(m_Value(A))))
-        return SelectInst::Create(cast<User>(A)->getOperand(0), C, B);
-      if (match(B, m_Not(m_Value(A))))
-        return SelectInst::Create(cast<User>(A)->getOperand(0), C, D);
+      if (match(D, m_Not(SELECT_MATCH(A))))
+        return SelectInst::Create(GET_SELECT_COND(D), C, B);
+      if (match(B, m_Not(SELECT_MATCH(A))))
+        return SelectInst::Create(GET_SELECT_COND(B), C, D);
     }
     if (match(B, m_Select(m_Value(), m_ConstantInt(-1), m_ConstantInt(0)))) {
-      if (match(C, m_Not(m_Value(B))))
-        return SelectInst::Create(cast<User>(B)->getOperand(0), A, D);
-      if (match(A, m_Not(m_Value(B))))
-        return SelectInst::Create(cast<User>(B)->getOperand(0), C, D);
+      if (match(C, m_Not(SELECT_MATCH(B))))
+        return SelectInst::Create(GET_SELECT_COND(C), A, D);
+      if (match(A, m_Not(SELECT_MATCH(B))))
+        return SelectInst::Create(GET_SELECT_COND(A), C, D);
     }
     if (match(C, m_Select(m_Value(), m_ConstantInt(-1), m_ConstantInt(0)))) {
-      if (match(D, m_Not(m_Value(C))))
-        return SelectInst::Create(cast<User>(C)->getOperand(0), A, B);
-      if (match(B, m_Not(m_Value(C))))
-        return SelectInst::Create(cast<User>(C)->getOperand(0), A, D);
+      if (match(D, m_Not(SELECT_MATCH(C))))
+        return SelectInst::Create(GET_SELECT_COND(D), A, B);
+      if (match(B, m_Not(SELECT_MATCH(C))))
+        return SelectInst::Create(GET_SELECT_COND(B), A, D);
     }
     if (match(D, m_Select(m_Value(), m_ConstantInt(-1), m_ConstantInt(0)))) {
-      if (match(C, m_Not(m_Value(D))))
-        return SelectInst::Create(cast<User>(D)->getOperand(0), A, B);
-      if (match(A, m_Not(m_Value(D))))
-        return SelectInst::Create(cast<User>(D)->getOperand(0), C, B);
+      if (match(C, m_Not(SELECT_MATCH(D))))
+        return SelectInst::Create(GET_SELECT_COND(C), A, B);
+      if (match(A, m_Not(SELECT_MATCH(D))))
+        return SelectInst::Create(GET_SELECT_COND(A), C, B);
     }
     if (match(A, m_Select(m_Value(), m_ConstantInt(0), m_ConstantInt(-1)))) {
-      if (match(D, m_Not(m_Value(A))))
-        return SelectInst::Create(cast<User>(A)->getOperand(0), B, C);
-      if (match(B, m_Not(m_Value(A))))
-        return SelectInst::Create(cast<User>(A)->getOperand(0), D, C);
+      if (match(D, m_Not(SELECT_MATCH(A))))
+        return SelectInst::Create(GET_SELECT_COND(D), B, C);
+      if (match(B, m_Not(SELECT_MATCH(A))))
+        return SelectInst::Create(GET_SELECT_COND(B), D, C);
     }
     if (match(B, m_Select(m_Value(), m_ConstantInt(0), m_ConstantInt(-1)))) {
-      if (match(C, m_Not(m_Value(B))))
-        return SelectInst::Create(cast<User>(B)->getOperand(0), D, A);
-      if (match(A, m_Not(m_Value(B))))
-        return SelectInst::Create(cast<User>(B)->getOperand(0), D, C);
+      if (match(C, m_Not(SELECT_MATCH(B))))
+        return SelectInst::Create(GET_SELECT_COND(C), D, A);
+      if (match(A, m_Not(SELECT_MATCH(B))))
+        return SelectInst::Create(GET_SELECT_COND(A), D, C);
     }
     if (match(C, m_Select(m_Value(), m_ConstantInt(0), m_ConstantInt(-1)))) {
-      if (match(D, m_Not(m_Value(C))))
-        return SelectInst::Create(cast<User>(C)->getOperand(0), B, A);
-      if (match(B, m_Not(m_Value(C))))
-        return SelectInst::Create(cast<User>(C)->getOperand(0), D, A);
+      if (match(D, m_Not(SELECT_MATCH(C))))
+        return SelectInst::Create(GET_SELECT_COND(D), B, A);
+      if (match(B, m_Not(SELECT_MATCH(C))))
+        return SelectInst::Create(GET_SELECT_COND(B), D, A);
     }
     if (match(D, m_Select(m_Value(), m_ConstantInt(0), m_ConstantInt(-1)))) {
-      if (match(C, m_Not(m_Value(D))))
-        return SelectInst::Create(cast<User>(D)->getOperand(0), B, A);
-      if (match(A, m_Not(m_Value(D))))
-        return SelectInst::Create(cast<User>(D)->getOperand(0), B, C);
+      if (match(C, m_Not(SELECT_MATCH(D))))
+        return SelectInst::Create(GET_SELECT_COND(C), B, A);
+      if (match(A, m_Not(SELECT_MATCH(D))))
+        return SelectInst::Create(GET_SELECT_COND(A), B, C);
     }
+#undef SELECT_MATCH
+#undef GET_SELECT_COND
   }
   
   // (X >> Z) | (Y >> Z)  -> (X|Y) >> Z  for all shifts.

Added: llvm/trunk/test/FrontendC/2008-11-08-InstCombineSelect.c
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/FrontendC/2008-11-08-InstCombineSelect.c?rev=58946&view=auto

==============================================================================
--- llvm/trunk/test/FrontendC/2008-11-08-InstCombineSelect.c (added)
+++ llvm/trunk/test/FrontendC/2008-11-08-InstCombineSelect.c Sun Nov  9 17:17:42 2008
@@ -0,0 +1,17 @@
+// RUN: %llvmgcc %s -S -emit-llvm -O2 -o -
+// PR3028
+
+int g_187;
+int g_204;
+int g_434;
+
+int func_89 (void)
+{
+  return 1;
+}
+
+void func_20 (int p_22)
+{
+  if (1 & p_22 | g_204 & (1 < g_187) - func_89 ())
+    g_434 = 1;
+}





More information about the llvm-commits mailing list