From 2e81087d24ada7f989e76738279ee5a2d7c04ded Mon Sep 17 00:00:00 2001 From: Mehrnoosh Heidarpour Date: Wed, 14 Apr 2021 20:49:46 +0200 Subject: [PATCH] [InstCombine] Conditionally emit nowrap flags when combining two adds Currently, the InstCombineCompare is combining two add operations into a single add operation which always has a nsw flag, without checking the conditions to see if this flag should be present according to the original two add operations or not. This patch will change the InstCombineCompare to emit the nsw or nuw only when these flags are allowed to be generated according to the original add operations and remove the possibility of applying wrong optimization with passes that will perform on the IR later in the pipeline. To confirm that the current results are buggy and the results after proposed patch are the correct IR the following examples from Alive2 are attached; the same results can be seen in the case of nuw flag and nsw is just used as an example. The following link shows that the generated IR with current LLVM is a buggy IR when none of the original add operations have nsw flag. https://alive2.llvm.org/ce/z/WGaDrm The following link proves that the generated IR after the patch in the former case is the correct IR. https://alive2.llvm.org/ce/z/wQ7G_e Differential Revision: https://reviews.llvm.org/D100095 --- .../InstCombine/InstCombineCompares.cpp | 8 +++++-- test/Transforms/InstCombine/icmp-add.ll | 22 ++++++++++++++----- 2 files changed, 23 insertions(+), 7 deletions(-) diff --git a/lib/Transforms/InstCombine/InstCombineCompares.cpp b/lib/Transforms/InstCombine/InstCombineCompares.cpp index 14748308439..fda87286fb3 100644 --- a/lib/Transforms/InstCombine/InstCombineCompares.cpp +++ b/lib/Transforms/InstCombine/InstCombineCompares.cpp @@ -3917,11 +3917,15 @@ Instruction *InstCombinerImpl::foldICmpBinOp(ICmpInst &I, APInt AP2Abs = C2->getValue().abs(); if (AP1Abs.uge(AP2Abs)) { ConstantInt *C3 = Builder.getInt(AP1 - AP2); - Value *NewAdd = Builder.CreateNSWAdd(A, C3); + bool HasNUW = BO0->hasNoUnsignedWrap() && C3->getValue().ule(AP1); + bool HasNSW = BO0->hasNoSignedWrap(); + Value *NewAdd = Builder.CreateAdd(A, C3, "", HasNUW, HasNSW); return new ICmpInst(Pred, NewAdd, C); } else { ConstantInt *C3 = Builder.getInt(AP2 - AP1); - Value *NewAdd = Builder.CreateNSWAdd(C, C3); + bool HasNUW = BO1->hasNoUnsignedWrap() && C3->getValue().ule(AP2); + bool HasNSW = BO1->hasNoSignedWrap(); + Value *NewAdd = Builder.CreateAdd(C, C3, "", HasNUW, HasNSW); return new ICmpInst(Pred, A, NewAdd); } } diff --git a/test/Transforms/InstCombine/icmp-add.ll b/test/Transforms/InstCombine/icmp-add.ll index c330cbc202a..a35d546f23a 100644 --- a/test/Transforms/InstCombine/icmp-add.ll +++ b/test/Transforms/InstCombine/icmp-add.ll @@ -666,7 +666,7 @@ define <2 x i1> @icmp_eq_add_non_splat2(<2 x i32> %a) { define i1 @without_nsw_nuw(i8 %x, i8 %y) { ; CHECK-LABEL: @without_nsw_nuw( -; CHECK-NEXT: [[TMP1:%.*]] = add nsw i8 [[X:%.*]], 2 +; CHECK-NEXT: [[TMP1:%.*]] = add i8 [[X:%.*]], 2 ; CHECK-NEXT: [[TOBOOL:%.*]] = icmp eq i8 [[TMP1]], [[Y:%.*]] ; CHECK-NEXT: ret i1 [[TOBOOL]] ; @@ -678,7 +678,7 @@ define i1 @without_nsw_nuw(i8 %x, i8 %y) { define i1 @with_nsw_nuw(i8 %x, i8 %y) { ; CHECK-LABEL: @with_nsw_nuw( -; CHECK-NEXT: [[TMP1:%.*]] = add nsw i8 [[X:%.*]], 2 +; CHECK-NEXT: [[TMP1:%.*]] = add nuw nsw i8 [[X:%.*]], 2 ; CHECK-NEXT: [[TOBOOL:%.*]] = icmp eq i8 [[TMP1]], [[Y:%.*]] ; CHECK-NEXT: ret i1 [[TOBOOL]] ; @@ -702,7 +702,7 @@ define i1 @with_nsw_large(i8 %x, i8 %y) { define i1 @with_nsw_small(i8 %x, i8 %y) { ; CHECK-LABEL: @with_nsw_small( -; CHECK-NEXT: [[TMP1:%.*]] = add nsw i8 [[Y:%.*]], 2 +; CHECK-NEXT: [[TMP1:%.*]] = add i8 [[Y:%.*]], 2 ; CHECK-NEXT: [[TOBOOL:%.*]] = icmp eq i8 [[TMP1]], [[X:%.*]] ; CHECK-NEXT: ret i1 [[TOBOOL]] ; @@ -714,7 +714,7 @@ define i1 @with_nsw_small(i8 %x, i8 %y) { define i1 @with_nuw_large(i8 %x, i8 %y) { ; CHECK-LABEL: @with_nuw_large( -; CHECK-NEXT: [[TMP1:%.*]] = add nsw i8 [[X:%.*]], 2 +; CHECK-NEXT: [[TMP1:%.*]] = add nuw i8 [[X:%.*]], 2 ; CHECK-NEXT: [[TOBOOL:%.*]] = icmp eq i8 [[TMP1]], [[Y:%.*]] ; CHECK-NEXT: ret i1 [[TOBOOL]] ; @@ -726,7 +726,7 @@ define i1 @with_nuw_large(i8 %x, i8 %y) { define i1 @with_nuw_small(i8 %x, i8 %y) { ; CHECK-LABEL: @with_nuw_small( -; CHECK-NEXT: [[TMP1:%.*]] = add nsw i8 [[Y:%.*]], 2 +; CHECK-NEXT: [[TMP1:%.*]] = add i8 [[Y:%.*]], 2 ; CHECK-NEXT: [[TOBOOL:%.*]] = icmp eq i8 [[TMP1]], [[X:%.*]] ; CHECK-NEXT: ret i1 [[TOBOOL]] ; @@ -735,3 +735,15 @@ define i1 @with_nuw_small(i8 %x, i8 %y) { %tobool = icmp eq i8 %t2, %t1 ret i1 %tobool } + +define i1 @with_nuw_large_negative(i8 %x, i8 %y) { +; CHECK-LABEL: @with_nuw_large_negative( +; CHECK-NEXT: [[TMP1:%.*]] = add i8 [[X:%.*]], -2 +; CHECK-NEXT: [[TOBOOL:%.*]] = icmp eq i8 [[TMP1]], [[Y:%.*]] +; CHECK-NEXT: ret i1 [[TOBOOL]] +; + %t1 = add nuw i8 %x, -37 + %t2 = add i8 %y, -35 + %tobool = icmp eq i8 %t2, %t1 + ret i1 %tobool +}