From 5c50ab84b32d52f7053a53d2b76f95022e7c3ef3 Mon Sep 17 00:00:00 2001 From: Jingyue Wu Date: Sat, 25 Oct 2014 17:36:21 +0000 Subject: [PATCH] [SeparateConstOffsetFromGEP] Fixed a bug in rebuilding OR expressions The two operands of the new OR expression should be NextInChain and TheOther instead of the two original operands. Added a regression test in split-gep.ll. Hao Liu reported this bug, and provded the test case and an initial patch. Thanks! git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@220615 91177308-0d34-0410-b5e6-96231b3b80d8 --- .../Scalar/SeparateConstOffsetFromGEP.cpp | 9 +++++++-- .../NVPTX/split-gep.ll | 19 +++++++++++++++++++ 2 files changed, 26 insertions(+), 2 deletions(-) diff --git a/lib/Transforms/Scalar/SeparateConstOffsetFromGEP.cpp b/lib/Transforms/Scalar/SeparateConstOffsetFromGEP.cpp index 6557ce4575d..449198c1d2f 100644 --- a/lib/Transforms/Scalar/SeparateConstOffsetFromGEP.cpp +++ b/lib/Transforms/Scalar/SeparateConstOffsetFromGEP.cpp @@ -519,8 +519,13 @@ Value *ConstantOffsetExtractor::removeConstOffset(unsigned ChainIndex) { // // Replacing the "or" with "add" is fine, because // a | (b + 5) = a + (b + 5) = (a + b) + 5 - return BinaryOperator::CreateAdd(BO->getOperand(0), BO->getOperand(1), - BO->getName(), IP); + if (OpNo == 0) { + return BinaryOperator::CreateAdd(NextInChain, TheOther, BO->getName(), + IP); + } else { + return BinaryOperator::CreateAdd(TheOther, NextInChain, BO->getName(), + IP); + } } // We can reuse BO in this case, because the new expression shares the same diff --git a/test/Transforms/SeparateConstOffsetFromGEP/NVPTX/split-gep.ll b/test/Transforms/SeparateConstOffsetFromGEP/NVPTX/split-gep.ll index 1784171454d..4d07ab35b51 100644 --- a/test/Transforms/SeparateConstOffsetFromGEP/NVPTX/split-gep.ll +++ b/test/Transforms/SeparateConstOffsetFromGEP/NVPTX/split-gep.ll @@ -234,3 +234,22 @@ entry: ; CHECK-LABEL: @and( ; CHECK: getelementptr [32 x [32 x float]]* @float_2d_array ; CHECK-NOT: getelementptr + +; The code that rebuilds an OR expression used to be buggy, and failed on this +; test. +define float* @shl_add_or(i64 %a, float* %ptr) { +; CHECK-LABEL: @shl_add_or( +entry: + %shl = shl i64 %a, 2 + %add = add i64 %shl, 12 + %or = or i64 %add, 1 +; CHECK: [[OR:%or[0-9]*]] = add i64 %shl, 1 + ; ((a << 2) + 12) and 1 have no common bits. Therefore, + ; SeparateConstOffsetFromGEP is able to extract the 12. + ; TODO(jingyue): We could reassociate the expression to combine 12 and 1. + %p = getelementptr float* %ptr, i64 %or +; CHECK: [[PTR:%[a-zA-Z0-9]+]] = getelementptr float* %ptr, i64 [[OR]] +; CHECK: getelementptr float* [[PTR]], i64 12 + ret float* %p +; CHECK-NEXT: ret +}