Skip to content

Conversation

@dtcxzyw
Copy link
Member

@dtcxzyw dtcxzyw commented Jan 4, 2025

@dtcxzyw dtcxzyw requested a review from goldsteinn January 4, 2025 07:56
@dtcxzyw dtcxzyw requested a review from nikic as a code owner January 4, 2025 07:56
@llvmbot llvmbot added llvm:instcombine Covers the InstCombine, InstSimplify and AggressiveInstCombine passes llvm:transforms labels Jan 4, 2025
@llvmbot
Copy link
Member

llvmbot commented Jan 4, 2025

@llvm/pr-subscribers-llvm-transforms

Author: Yingwei Zheng (dtcxzyw)

Changes

Alive2: https://alive2.llvm.org/ce/z/Dr3Sbe
Closes #121581.


Full diff: https://github.com/llvm/llvm-project/pull/121628.diff

2 Files Affected:

  • (modified) llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp (+4-2)
  • (modified) llvm/test/Transforms/InstCombine/icmp-gep.ll (+48)
diff --git a/llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp b/llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp
index d6fdade25559fe..8b23583c510637 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineCompares.cpp
@@ -747,6 +747,8 @@ Instruction *InstCombinerImpl::foldGEPICmp(GEPOperator *GEPLHS, Value *RHS,
                         ConstantExpr::getPointerBitCastOrAddrSpaceCast(
                             cast<Constant>(RHS), Base->getType()));
   } else if (GEPOperator *GEPRHS = dyn_cast<GEPOperator>(RHS)) {
+    GEPNoWrapFlags NW = GEPLHS->getNoWrapFlags() & GEPRHS->getNoWrapFlags();
+
     // If the base pointers are different, but the indices are the same, just
     // compare the base pointer.
     if (PtrBase != GEPRHS->getOperand(0)) {
@@ -764,7 +766,8 @@ Instruction *InstCombinerImpl::foldGEPICmp(GEPOperator *GEPLHS, Value *RHS,
 
       // If all indices are the same, just compare the base pointers.
       Type *BaseType = GEPLHS->getOperand(0)->getType();
-      if (IndicesTheSame && CmpInst::makeCmpResultType(BaseType) == I.getType())
+      if (IndicesTheSame &&
+          CmpInst::makeCmpResultType(BaseType) == I.getType() && CanFold(NW))
         return new ICmpInst(Cond, GEPLHS->getOperand(0), GEPRHS->getOperand(0));
 
       // If we're comparing GEPs with two base pointers that only differ in type
@@ -804,7 +807,6 @@ Instruction *InstCombinerImpl::foldGEPICmp(GEPOperator *GEPLHS, Value *RHS,
       return transformToIndexedCompare(GEPLHS, RHS, Cond, DL, *this);
     }
 
-    GEPNoWrapFlags NW = GEPLHS->getNoWrapFlags() & GEPRHS->getNoWrapFlags();
     if (GEPLHS->getNumOperands() == GEPRHS->getNumOperands() &&
         GEPLHS->getSourceElementType() == GEPRHS->getSourceElementType()) {
       // If the GEPs only differ by one index, compare it.
diff --git a/llvm/test/Transforms/InstCombine/icmp-gep.ll b/llvm/test/Transforms/InstCombine/icmp-gep.ll
index f9b90c224d8327..7f8f1ae73948d1 100644
--- a/llvm/test/Transforms/InstCombine/icmp-gep.ll
+++ b/llvm/test/Transforms/InstCombine/icmp-gep.ll
@@ -709,3 +709,51 @@ define i1 @pointer_icmp_aligned_with_offset_negative(ptr align 8 %a, ptr align 8
   %cmp = icmp eq ptr %gep, %a2
   ret i1 %cmp
 }
+
+define i1 @gep_diff_base_same_indices(ptr %x, ptr %y, i64 %z) {
+; CHECK-LABEL: @gep_diff_base_same_indices(
+; CHECK-NEXT:    [[X:%.*]] = getelementptr i8, ptr [[X1:%.*]], i64 [[Z:%.*]]
+; CHECK-NEXT:    [[Y:%.*]] = getelementptr i8, ptr [[Y1:%.*]], i64 [[Z]]
+; CHECK-NEXT:    [[CMP:%.*]] = icmp ult ptr [[X]], [[Y]]
+; CHECK-NEXT:    ret i1 [[CMP]]
+;
+  %gep1 = getelementptr i8, ptr %x, i64 %z
+  %gep2 = getelementptr i8, ptr %y, i64 %z
+  %cmp = icmp ult ptr %gep1, %gep2
+  ret i1 %cmp
+}
+
+define i1 @gep_diff_base_same_indices_nuw(ptr %x, ptr %y, i64 %z) {
+; CHECK-LABEL: @gep_diff_base_same_indices_nuw(
+; CHECK-NEXT:    [[CMP:%.*]] = icmp ult ptr [[X:%.*]], [[Y:%.*]]
+; CHECK-NEXT:    ret i1 [[CMP]]
+;
+  %gep1 = getelementptr nuw i8, ptr %x, i64 %z
+  %gep2 = getelementptr nuw i8, ptr %y, i64 %z
+  %cmp = icmp ult ptr %gep1, %gep2
+  ret i1 %cmp
+}
+
+define i1 @gep_diff_base_same_indices_nusw(ptr %x, ptr %y, i64 %z) {
+; CHECK-LABEL: @gep_diff_base_same_indices_nusw(
+; CHECK-NEXT:    [[CMP:%.*]] = icmp ult ptr [[X:%.*]], [[Y:%.*]]
+; CHECK-NEXT:    ret i1 [[CMP]]
+;
+  %gep1 = getelementptr nusw i8, ptr %x, i64 %z
+  %gep2 = getelementptr nusw i8, ptr %y, i64 %z
+  %cmp = icmp ult ptr %gep1, %gep2
+  ret i1 %cmp
+}
+
+define i1 @gep_diff_base_same_indices_nuw_nusw(ptr %x, ptr %y, i64 %z) {
+; CHECK-LABEL: @gep_diff_base_same_indices_nuw_nusw(
+; CHECK-NEXT:    [[X:%.*]] = getelementptr nuw i8, ptr [[X1:%.*]], i64 [[Z:%.*]]
+; CHECK-NEXT:    [[Y:%.*]] = getelementptr nusw i8, ptr [[Y1:%.*]], i64 [[Z]]
+; CHECK-NEXT:    [[CMP:%.*]] = icmp ult ptr [[X]], [[Y]]
+; CHECK-NEXT:    ret i1 [[CMP]]
+;
+  %gep1 = getelementptr nuw i8, ptr %x, i64 %z
+  %gep2 = getelementptr nusw i8, ptr %y, i64 %z
+  %cmp = icmp ult ptr %gep1, %gep2
+  ret i1 %cmp
+}

Copy link
Contributor

@nikic nikic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!

@dtcxzyw dtcxzyw merged commit fac4646 into llvm:main Jan 4, 2025
11 checks passed
@dtcxzyw dtcxzyw deleted the icmp-gep-nw-flag branch January 4, 2025 09:23
@llvm-ci
Copy link
Collaborator

llvm-ci commented Jan 4, 2025

LLVM Buildbot has detected a new failure on builder clang-aarch64-sve2-vla running on linaro-g4-01 while building llvm at step 7 "ninja check 1".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/198/builds/812

Here is the relevant piece of the build log for the reference
Step 7 (ninja check 1) failure: stage 1 checked (failure)
******************** TEST 'MLIR :: Integration/Dialect/SparseTensor/CPU/sparse_block_matmul.mlir' FAILED ********************
Exit Code: 2

Command Output (stdout):
--
# RUN: at line 21
/home/tcwg-buildbot/worker/clang-aarch64-sve2-vla/stage1/bin/mlir-opt /home/tcwg-buildbot/worker/clang-aarch64-sve2-vla/llvm/mlir/test/Integration/Dialect/SparseTensor/CPU/sparse_block_matmul.mlir --sparsifier="enable-runtime-library=true" | /home/tcwg-buildbot/worker/clang-aarch64-sve2-vla/stage1/bin/mlir-cpu-runner -e main -entry-point-result=void -shared-libs=/home/tcwg-buildbot/worker/clang-aarch64-sve2-vla/stage1/lib/libmlir_c_runner_utils.so,/home/tcwg-buildbot/worker/clang-aarch64-sve2-vla/stage1/lib/libmlir_runner_utils.so | /home/tcwg-buildbot/worker/clang-aarch64-sve2-vla/stage1/bin/FileCheck /home/tcwg-buildbot/worker/clang-aarch64-sve2-vla/llvm/mlir/test/Integration/Dialect/SparseTensor/CPU/sparse_block_matmul.mlir
# executed command: /home/tcwg-buildbot/worker/clang-aarch64-sve2-vla/stage1/bin/mlir-opt /home/tcwg-buildbot/worker/clang-aarch64-sve2-vla/llvm/mlir/test/Integration/Dialect/SparseTensor/CPU/sparse_block_matmul.mlir --sparsifier=enable-runtime-library=true
# executed command: /home/tcwg-buildbot/worker/clang-aarch64-sve2-vla/stage1/bin/mlir-cpu-runner -e main -entry-point-result=void -shared-libs=/home/tcwg-buildbot/worker/clang-aarch64-sve2-vla/stage1/lib/libmlir_c_runner_utils.so,/home/tcwg-buildbot/worker/clang-aarch64-sve2-vla/stage1/lib/libmlir_runner_utils.so
# executed command: /home/tcwg-buildbot/worker/clang-aarch64-sve2-vla/stage1/bin/FileCheck /home/tcwg-buildbot/worker/clang-aarch64-sve2-vla/llvm/mlir/test/Integration/Dialect/SparseTensor/CPU/sparse_block_matmul.mlir
# RUN: at line 25
/home/tcwg-buildbot/worker/clang-aarch64-sve2-vla/stage1/bin/mlir-opt /home/tcwg-buildbot/worker/clang-aarch64-sve2-vla/llvm/mlir/test/Integration/Dialect/SparseTensor/CPU/sparse_block_matmul.mlir --sparsifier="enable-runtime-library=false" | /home/tcwg-buildbot/worker/clang-aarch64-sve2-vla/stage1/bin/mlir-cpu-runner -e main -entry-point-result=void -shared-libs=/home/tcwg-buildbot/worker/clang-aarch64-sve2-vla/stage1/lib/libmlir_c_runner_utils.so,/home/tcwg-buildbot/worker/clang-aarch64-sve2-vla/stage1/lib/libmlir_runner_utils.so | /home/tcwg-buildbot/worker/clang-aarch64-sve2-vla/stage1/bin/FileCheck /home/tcwg-buildbot/worker/clang-aarch64-sve2-vla/llvm/mlir/test/Integration/Dialect/SparseTensor/CPU/sparse_block_matmul.mlir
# executed command: /home/tcwg-buildbot/worker/clang-aarch64-sve2-vla/stage1/bin/mlir-opt /home/tcwg-buildbot/worker/clang-aarch64-sve2-vla/llvm/mlir/test/Integration/Dialect/SparseTensor/CPU/sparse_block_matmul.mlir --sparsifier=enable-runtime-library=false
# executed command: /home/tcwg-buildbot/worker/clang-aarch64-sve2-vla/stage1/bin/mlir-cpu-runner -e main -entry-point-result=void -shared-libs=/home/tcwg-buildbot/worker/clang-aarch64-sve2-vla/stage1/lib/libmlir_c_runner_utils.so,/home/tcwg-buildbot/worker/clang-aarch64-sve2-vla/stage1/lib/libmlir_runner_utils.so
# executed command: /home/tcwg-buildbot/worker/clang-aarch64-sve2-vla/stage1/bin/FileCheck /home/tcwg-buildbot/worker/clang-aarch64-sve2-vla/llvm/mlir/test/Integration/Dialect/SparseTensor/CPU/sparse_block_matmul.mlir
# RUN: at line 30
/home/tcwg-buildbot/worker/clang-aarch64-sve2-vla/stage1/bin/mlir-opt /home/tcwg-buildbot/worker/clang-aarch64-sve2-vla/llvm/mlir/test/Integration/Dialect/SparseTensor/CPU/sparse_block_matmul.mlir --sparsifier="enable-runtime-library=false vl=2 reassociate-fp-reductions=true enable-index-optimizations=true" | /home/tcwg-buildbot/worker/clang-aarch64-sve2-vla/stage1/bin/mlir-cpu-runner -e main -entry-point-result=void -shared-libs=/home/tcwg-buildbot/worker/clang-aarch64-sve2-vla/stage1/lib/libmlir_c_runner_utils.so,/home/tcwg-buildbot/worker/clang-aarch64-sve2-vla/stage1/lib/libmlir_runner_utils.so | /home/tcwg-buildbot/worker/clang-aarch64-sve2-vla/stage1/bin/FileCheck /home/tcwg-buildbot/worker/clang-aarch64-sve2-vla/llvm/mlir/test/Integration/Dialect/SparseTensor/CPU/sparse_block_matmul.mlir
# executed command: /home/tcwg-buildbot/worker/clang-aarch64-sve2-vla/stage1/bin/mlir-opt /home/tcwg-buildbot/worker/clang-aarch64-sve2-vla/llvm/mlir/test/Integration/Dialect/SparseTensor/CPU/sparse_block_matmul.mlir '--sparsifier=enable-runtime-library=false vl=2 reassociate-fp-reductions=true enable-index-optimizations=true'
# executed command: /home/tcwg-buildbot/worker/clang-aarch64-sve2-vla/stage1/bin/mlir-cpu-runner -e main -entry-point-result=void -shared-libs=/home/tcwg-buildbot/worker/clang-aarch64-sve2-vla/stage1/lib/libmlir_c_runner_utils.so,/home/tcwg-buildbot/worker/clang-aarch64-sve2-vla/stage1/lib/libmlir_runner_utils.so
# executed command: /home/tcwg-buildbot/worker/clang-aarch64-sve2-vla/stage1/bin/FileCheck /home/tcwg-buildbot/worker/clang-aarch64-sve2-vla/llvm/mlir/test/Integration/Dialect/SparseTensor/CPU/sparse_block_matmul.mlir
# RUN: at line 33
/home/tcwg-buildbot/worker/clang-aarch64-sve2-vla/stage1/bin/mlir-opt /home/tcwg-buildbot/worker/clang-aarch64-sve2-vla/llvm/mlir/test/Integration/Dialect/SparseTensor/CPU/sparse_block_matmul.mlir --sparsifier="enable-arm-sve=true enable-runtime-library=false vl=2 reassociate-fp-reductions=true enable-index-optimizations=true" | mlir-cpu-runner --march=aarch64 --mattr="+sve" -e main -entry-point-result=void -shared-libs=/home/tcwg-buildbot/worker/clang-aarch64-sve2-vla/stage1/./lib/libmlir_runner_utils.so,/home/tcwg-buildbot/worker/clang-aarch64-sve2-vla/stage1/./lib/libmlir_c_runner_utils.so | /home/tcwg-buildbot/worker/clang-aarch64-sve2-vla/stage1/bin/FileCheck /home/tcwg-buildbot/worker/clang-aarch64-sve2-vla/llvm/mlir/test/Integration/Dialect/SparseTensor/CPU/sparse_block_matmul.mlir
# executed command: /home/tcwg-buildbot/worker/clang-aarch64-sve2-vla/stage1/bin/mlir-opt /home/tcwg-buildbot/worker/clang-aarch64-sve2-vla/llvm/mlir/test/Integration/Dialect/SparseTensor/CPU/sparse_block_matmul.mlir '--sparsifier=enable-arm-sve=true enable-runtime-library=false vl=2 reassociate-fp-reductions=true enable-index-optimizations=true'
# .---command stderr------------
# | /home/tcwg-buildbot/worker/clang-aarch64-sve2-vla/llvm/mlir/test/Integration/Dialect/SparseTensor/CPU/sparse_block_matmul.mlir:71:10: error: null operand found
# |     %0 = linalg.generic #trait_mul
# |          ^
# | /home/tcwg-buildbot/worker/clang-aarch64-sve2-vla/llvm/mlir/test/Integration/Dialect/SparseTensor/CPU/sparse_block_matmul.mlir:71:10: note: see current operation: %72 = "arith.select"(<<NULL VALUE>>, %71, %49) : (<<NULL TYPE>>, vector<[2]xf64>, vector<[2]xf64>) -> vector<[2]xf64>
# `-----------------------------
# error: command failed with exit status: 1
# executed command: mlir-cpu-runner --march=aarch64 --mattr=+sve -e main -entry-point-result=void -shared-libs=/home/tcwg-buildbot/worker/clang-aarch64-sve2-vla/stage1/./lib/libmlir_runner_utils.so,/home/tcwg-buildbot/worker/clang-aarch64-sve2-vla/stage1/./lib/libmlir_c_runner_utils.so
# .---command stderr------------
# | Error: entry point not found
# `-----------------------------
# error: command failed with exit status: 1
# executed command: /home/tcwg-buildbot/worker/clang-aarch64-sve2-vla/stage1/bin/FileCheck /home/tcwg-buildbot/worker/clang-aarch64-sve2-vla/llvm/mlir/test/Integration/Dialect/SparseTensor/CPU/sparse_block_matmul.mlir
# .---command stderr------------
# | FileCheck error: '<stdin>' is empty.
# | FileCheck command line:  /home/tcwg-buildbot/worker/clang-aarch64-sve2-vla/stage1/bin/FileCheck /home/tcwg-buildbot/worker/clang-aarch64-sve2-vla/llvm/mlir/test/Integration/Dialect/SparseTensor/CPU/sparse_block_matmul.mlir
# `-----------------------------
# error: command failed with exit status: 2

--

********************


Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

llvm:instcombine Covers the InstCombine, InstSimplify and AggressiveInstCombine passes llvm:transforms

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[InstCombine] Incorrect fold of the comparison of GEPs

4 participants