Skip to content

Commit

Permalink
[Analysis] remove bogus smin/smax pattern detection
Browse files Browse the repository at this point in the history 
This is a revert of cfcc42b. The analysis is wrong as shown by
the minimal tests for instcombine:
https://alive2.llvm.org/ce/z/y9Dp8A

There may be a way to salvage some of the other tests,
but that can be done as follow-ups. This avoids a miscompile
and fixes #54311.
  • Loading branch information
@rotateright
rotateright committed Mar 9, 2022
1 parent cc8f981 commit b48fe15
Show file tree
Hide file tree
Showing 5 changed files with 36 additions and 40 deletions.
14 changes: 0 additions & 14 deletions llvm/lib/Analysis/ValueTracking.cpp
View file
Expand Up @@ -5802,20 +5802,6 @@ static SelectPatternResult matchMinMax(CmpInst::Predicate Pred,
if (Pred != CmpInst::ICMP_SGT && Pred != CmpInst::ICMP_SLT)
return {SPF_UNKNOWN, SPNB_NA, false};

// Z = X -nsw Y
// (X >s Y) ? 0 : Z ==> (Z >s 0) ? 0 : Z ==> SMIN(Z, 0)
// (X <s Y) ? 0 : Z ==> (Z <s 0) ? 0 : Z ==> SMAX(Z, 0)
if (match(TrueVal, m_Zero()) &&
match(FalseVal, m_NSWSub(m_Specific(CmpLHS), m_Specific(CmpRHS))))
return {Pred == CmpInst::ICMP_SGT ? SPF_SMIN : SPF_SMAX, SPNB_NA, false};

// Z = X -nsw Y
// (X >s Y) ? Z : 0 ==> (Z >s 0) ? Z : 0 ==> SMAX(Z, 0)
// (X <s Y) ? Z : 0 ==> (Z <s 0) ? Z : 0 ==> SMIN(Z, 0)
if (match(FalseVal, m_Zero()) &&
match(TrueVal, m_NSWSub(m_Specific(CmpLHS), m_Specific(CmpRHS))))
return {Pred == CmpInst::ICMP_SGT ? SPF_SMAX : SPF_SMIN, SPNB_NA, false};

const APInt *C1;
if (!match(CmpRHS, m_APInt(C1)))
return {SPF_UNKNOWN, SPNB_NA, false};
Expand Down
24 changes: 12 additions & 12 deletions llvm/test/CodeGen/X86/vec_minmax_match.ll
View file
Expand Up @@ -35,9 +35,9 @@ define <4 x i32> @smin_vec2(<4 x i32> %x) {
define <4 x i32> @smin_vec3(<4 x i32> %x, <4 x i32> %y) {
; CHECK-LABEL: smin_vec3:
; CHECK: # %bb.0:
; CHECK-NEXT: vpsubd %xmm1, %xmm0, %xmm0
; CHECK-NEXT: vpxor %xmm1, %xmm1, %xmm1
; CHECK-NEXT: vpminsd %xmm1, %xmm0, %xmm0
; CHECK-NEXT: vpsubd %xmm1, %xmm0, %xmm2
; CHECK-NEXT: vpcmpgtd %xmm1, %xmm0, %xmm0
; CHECK-NEXT: vpandn %xmm2, %xmm0, %xmm0
; CHECK-NEXT: retq
%sub = sub nsw <4 x i32> %x, %y
%cmp = icmp sgt <4 x i32> %x, %y
Expand All @@ -50,9 +50,9 @@ define <4 x i32> @smin_vec3(<4 x i32> %x, <4 x i32> %y) {
define <4 x i32> @smin_vec4(<4 x i32> %x, <4 x i32> %y) {
; CHECK-LABEL: smin_vec4:
; CHECK: # %bb.0:
; CHECK-NEXT: vpsubd %xmm1, %xmm0, %xmm0
; CHECK-NEXT: vpxor %xmm1, %xmm1, %xmm1
; CHECK-NEXT: vpminsd %xmm1, %xmm0, %xmm0
; CHECK-NEXT: vpsubd %xmm1, %xmm0, %xmm2
; CHECK-NEXT: vpcmpgtd %xmm0, %xmm1, %xmm0
; CHECK-NEXT: vpand %xmm2, %xmm0, %xmm0
; CHECK-NEXT: retq
%sub = sub nsw <4 x i32> %x, %y
%cmp = icmp slt <4 x i32> %x, %y
Expand Down Expand Up @@ -91,9 +91,9 @@ define <4 x i32> @smax_vec2(<4 x i32> %x) {
define <4 x i32> @smax_vec3(<4 x i32> %x, <4 x i32> %y) {
; CHECK-LABEL: smax_vec3:
; CHECK: # %bb.0:
; CHECK-NEXT: vpsubd %xmm1, %xmm0, %xmm0
; CHECK-NEXT: vpxor %xmm1, %xmm1, %xmm1
; CHECK-NEXT: vpmaxsd %xmm1, %xmm0, %xmm0
; CHECK-NEXT: vpsubd %xmm1, %xmm0, %xmm2
; CHECK-NEXT: vpcmpgtd %xmm0, %xmm1, %xmm0
; CHECK-NEXT: vpandn %xmm2, %xmm0, %xmm0
; CHECK-NEXT: retq
%sub = sub nsw <4 x i32> %x, %y
%cmp = icmp slt <4 x i32> %x, %y
Expand All @@ -106,9 +106,9 @@ define <4 x i32> @smax_vec3(<4 x i32> %x, <4 x i32> %y) {
define <4 x i32> @smax_vec4(<4 x i32> %x, <4 x i32> %y) {
; CHECK-LABEL: smax_vec4:
; CHECK: # %bb.0:
; CHECK-NEXT: vpsubd %xmm1, %xmm0, %xmm0
; CHECK-NEXT: vpxor %xmm1, %xmm1, %xmm1
; CHECK-NEXT: vpmaxsd %xmm1, %xmm0, %xmm0
; CHECK-NEXT: vpsubd %xmm1, %xmm0, %xmm2
; CHECK-NEXT: vpcmpgtd %xmm1, %xmm0, %xmm0
; CHECK-NEXT: vpand %xmm2, %xmm0, %xmm0
; CHECK-NEXT: retq
%sub = sub nsw <4 x i32> %x, %y
%cmp = icmp sgt <4 x i32> %x, %y
Expand Down
28 changes: 16 additions & 12 deletions llvm/test/Transforms/InstCombine/select-min-max.ll
View file
Expand Up @@ -252,9 +252,10 @@ define i8 @umin_umax(i8 %x) {

define i8 @not_smax(i8 %i41, i8 %i43) {
; CHECK-LABEL: @not_smax(
; CHECK-NEXT: [[I46:%.*]] = sub nsw i8 [[I41:%.*]], [[I43:%.*]]
; CHECK-NEXT: [[TMP1:%.*]] = call i8 @llvm.smax.i8(i8 [[I46]], i8 0)
; CHECK-NEXT: ret i8 [[TMP1]]
; CHECK-NEXT: [[I44:%.*]] = icmp slt i8 [[I41:%.*]], [[I43:%.*]]
; CHECK-NEXT: [[I46:%.*]] = sub nsw i8 [[I41]], [[I43]]
; CHECK-NEXT: [[SPEC_SELECT:%.*]] = select i1 [[I44]], i8 0, i8 [[I46]]
; CHECK-NEXT: ret i8 [[SPEC_SELECT]]
;
%i44 = icmp slt i8 %i41, %i43
%i46 = sub nsw i8 %i41, %i43
Expand All @@ -264,9 +265,10 @@ define i8 @not_smax(i8 %i41, i8 %i43) {

define i8 @not_smax_swap(i8 %i41, i8 %i43) {
; CHECK-LABEL: @not_smax_swap(
; CHECK-NEXT: [[I46:%.*]] = sub nsw i8 [[I41:%.*]], [[I43:%.*]]
; CHECK-NEXT: [[TMP1:%.*]] = call i8 @llvm.smax.i8(i8 [[I46]], i8 0)
; CHECK-NEXT: ret i8 [[TMP1]]
; CHECK-NEXT: [[I44:%.*]] = icmp sgt i8 [[I41:%.*]], [[I43:%.*]]
; CHECK-NEXT: [[I46:%.*]] = sub nsw i8 [[I41]], [[I43]]
; CHECK-NEXT: [[SPEC_SELECT:%.*]] = select i1 [[I44]], i8 [[I46]], i8 0
; CHECK-NEXT: ret i8 [[SPEC_SELECT]]
;
%i44 = icmp sgt i8 %i41, %i43
%i46 = sub nsw i8 %i41, %i43
Expand All @@ -276,9 +278,10 @@ define i8 @not_smax_swap(i8 %i41, i8 %i43) {

define i8 @not_smin(i8 %i41, i8 %i43) {
; CHECK-LABEL: @not_smin(
; CHECK-NEXT: [[I46:%.*]] = sub nsw i8 [[I41:%.*]], [[I43:%.*]]
; CHECK-NEXT: [[TMP1:%.*]] = call i8 @llvm.smin.i8(i8 [[I46]], i8 0)
; CHECK-NEXT: ret i8 [[TMP1]]
; CHECK-NEXT: [[I44:%.*]] = icmp sgt i8 [[I41:%.*]], [[I43:%.*]]
; CHECK-NEXT: [[I46:%.*]] = sub nsw i8 [[I41]], [[I43]]
; CHECK-NEXT: [[SPEC_SELECT:%.*]] = select i1 [[I44]], i8 0, i8 [[I46]]
; CHECK-NEXT: ret i8 [[SPEC_SELECT]]
;
%i44 = icmp sgt i8 %i41, %i43
%i46 = sub nsw i8 %i41, %i43
Expand All @@ -288,9 +291,10 @@ define i8 @not_smin(i8 %i41, i8 %i43) {

define i8 @not_smin_swap(i8 %i41, i8 %i43) {
; CHECK-LABEL: @not_smin_swap(
; CHECK-NEXT: [[I46:%.*]] = sub nsw i8 [[I41:%.*]], [[I43:%.*]]
; CHECK-NEXT: [[TMP1:%.*]] = call i8 @llvm.smin.i8(i8 [[I46]], i8 0)
; CHECK-NEXT: ret i8 [[TMP1]]
; CHECK-NEXT: [[I44:%.*]] = icmp slt i8 [[I41:%.*]], [[I43:%.*]]
; CHECK-NEXT: [[I46:%.*]] = sub nsw i8 [[I41]], [[I43]]
; CHECK-NEXT: [[SPEC_SELECT:%.*]] = select i1 [[I44]], i8 [[I46]], i8 0
; CHECK-NEXT: ret i8 [[SPEC_SELECT]]
;
%i44 = icmp slt i8 %i41, %i43
%i46 = sub nsw i8 %i41, %i43
Expand Down
3 changes: 2 additions & 1 deletion llvm/test/Transforms/LoopVectorize/bzip_reverse_loops.ll
View file
Expand Up @@ -40,7 +40,8 @@ do.end: ; preds = %cond.end
;CHECK: example1
;CHECK: load <4 x i32>
;CHECK-NEXT: shufflevector <4 x i32>
;CHECK: call <4 x i32> @llvm.smax.v4i32
;CHECK: sub nsw <4 x i32>
;CHECK: select <4 x i1>
;CHECK: store <4 x i32>
;CHECK: ret
define void @example1(i32* nocapture %a, i32 %n, i32 %wsize) nounwind uwtable ssp {
Expand Down
7 changes: 6 additions & 1 deletion llvm/test/Transforms/PhaseOrdering/min-max-abs-cse.ll
View file
Expand Up @@ -9,7 +9,12 @@

define i8 @smax_nsw(i8 %a, i8 %b) {
; CHECK-LABEL: @smax_nsw(
; CHECK-NEXT: ret i8 0
; CHECK-NEXT: [[SUB:%.*]] = sub nsw i8 [[A:%.*]], [[B:%.*]]
; CHECK-NEXT: [[CMP1:%.*]] = icmp slt i8 [[A]], [[B]]
; CHECK-NEXT: [[M1:%.*]] = select i1 [[CMP1]], i8 0, i8 [[SUB]]
; CHECK-NEXT: [[TMP1:%.*]] = call i8 @llvm.smax.i8(i8 [[SUB]], i8 0)
; CHECK-NEXT: [[R:%.*]] = sub i8 [[TMP1]], [[M1]]
; CHECK-NEXT: ret i8 [[R]]
;
%sub = sub nsw i8 %a, %b
%cmp1 = icmp slt i8 %a, %b
Expand Down

0 comments on commit b48fe15

Please sign in to comment.