From 1fb43c32edd3b3eb9e4ece9240578984c5914d65 Mon Sep 17 00:00:00 2001 From: Bill Wendling Date: Sun, 9 Nov 2008 23:17:42 +0000 Subject: [PATCH] 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. git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@58946 91177308-0d34-0410-b5e6-96231b3b80d8 --- .../Scalar/InstructionCombining.cpp | 71 ++++++++++--------- test/FrontendC/2008-11-08-InstCombineSelect.c | 17 +++++ 2 files changed, 56 insertions(+), 32 deletions(-) create mode 100644 test/FrontendC/2008-11-08-InstCombineSelect.c diff --git a/lib/Transforms/Scalar/InstructionCombining.cpp b/lib/Transforms/Scalar/InstructionCombining.cpp index 60b907e0da9..fa0704cc404 100644 --- a/lib/Transforms/Scalar/InstructionCombining.cpp +++ b/lib/Transforms/Scalar/InstructionCombining.cpp @@ -4352,55 +4352,62 @@ Instruction *InstCombiner::visitOr(BinaryOperator &I) { } } +#define SELECT_MATCH(Val) \ + m_Select(m_Value(Val), m_ConstantInt(0), m_ConstantInt(-1)) +#define GET_SELECT_COND(Val) \ + cast(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(A)->getOperand(0), C, B); - if (match(B, m_Not(m_Value(A)))) - return SelectInst::Create(cast(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(B)->getOperand(0), A, D); - if (match(A, m_Not(m_Value(B)))) - return SelectInst::Create(cast(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(C)->getOperand(0), A, B); - if (match(B, m_Not(m_Value(C)))) - return SelectInst::Create(cast(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(D)->getOperand(0), A, B); - if (match(A, m_Not(m_Value(D)))) - return SelectInst::Create(cast(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(A)->getOperand(0), B, C); - if (match(B, m_Not(m_Value(A)))) - return SelectInst::Create(cast(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(B)->getOperand(0), D, A); - if (match(A, m_Not(m_Value(B)))) - return SelectInst::Create(cast(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(C)->getOperand(0), B, A); - if (match(B, m_Not(m_Value(C)))) - return SelectInst::Create(cast(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(D)->getOperand(0), B, A); - if (match(A, m_Not(m_Value(D)))) - return SelectInst::Create(cast(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. diff --git a/test/FrontendC/2008-11-08-InstCombineSelect.c b/test/FrontendC/2008-11-08-InstCombineSelect.c new file mode 100644 index 00000000000..70c8d3a2248 --- /dev/null +++ b/test/FrontendC/2008-11-08-InstCombineSelect.c @@ -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; +}