Skip to content
Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions clang/test/CodeGen/SystemZ/builtins-systemz-zvector.c
Original file line number Diff line number Diff line change
Expand Up @@ -3584,13 +3584,13 @@ void test_integer(void) {
// CHECK-ASM: vsrlb

vsc = vec_abs(vsc);
// CHECK-ASM: vlcb
// CHECK-ASM: vlpb
Copy link
Member

Choose a reason for hiding this comment

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

The SystemZ changes LGTM. In fact, this fixes a regression I hadn't even been aware of, which was introduced here: 1a60ae0

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The SystemZ changes LGTM. In fact, this fixes a regression I hadn't even been aware of, which was introduced here: 1a60ae0

thanks @uweigand for review

vss = vec_abs(vss);
// CHECK-ASM: vlch
// CHECK-ASM: vlph
vsi = vec_abs(vsi);
// CHECK-ASM: vlcf
// CHECK-ASM: vlpf
vsl = vec_abs(vsl);
// CHECK-ASM: vlcg
// CHECK-ASM: vlpg

vsc = vec_max(vsc, vsc);
// CHECK-ASM: vmxb
Expand Down
2 changes: 1 addition & 1 deletion clang/test/CodeGen/SystemZ/builtins-systemz-zvector5.c
Original file line number Diff line number Diff line change
Expand Up @@ -246,7 +246,7 @@ void test_integer(void) {
// CHECK-ASM: vctzq

vslll = vec_abs(vslll);
// CHECK-ASM: vlcq
// CHECK-ASM: vlpq

vslll = vec_avg(vslll, vslll);
// CHECK: call i128 @llvm.s390.vavgq(i128 %{{.*}}, i128 %{{.*}})
Expand Down
24 changes: 24 additions & 0 deletions llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3997,6 +3997,27 @@ static Value *foldOrUnsignedUMulOverflowICmp(BinaryOperator &I,
return nullptr;
}

/// Fold select(X >s 0, 0, -X) | smax(X, 0) --> abs(X)
/// select(X <s 0, -X, 0) | smax(X, 0) --> abs(X)
static Value *FoldOrOfSelectSmaxToAbs(BinaryOperator &I,
InstCombiner::BuilderTy &Builder) {
Value *X;
Value *Sel;
if (match(&I, m_c_Or(m_Value(Sel), m_OneUse(m_Intrinsic<Intrinsic::smax>(
Copy link
Contributor

Choose a reason for hiding this comment

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

Use m_SMax

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Use m_SMax

done

m_Value(X), m_ZeroInt()))))) {
auto NegX = m_Neg(m_Specific(X));
if (match(Sel, m_Select(m_SpecificICmp(ICmpInst::ICMP_SGT, m_Specific(X),
m_ZeroInt()),
m_ZeroInt(), NegX)) ||
match(Sel, m_Select(m_SpecificICmp(ICmpInst::ICMP_SLT, m_Specific(X),
m_ZeroInt()),
NegX, m_ZeroInt())))
return Builder.CreateBinaryIntrinsic(Intrinsic::abs, X,
Builder.getFalse());
}
return nullptr;
}

// FIXME: We use commutative matchers (m_c_*) for some, but not all, matches
// here. We should standardize that construct where it is needed or choose some
// other way to ensure that commutated variants of patterns are not missed.
Expand Down Expand Up @@ -4545,6 +4566,9 @@ Instruction *InstCombinerImpl::visitOr(BinaryOperator &I) {
if (Value *V = SimplifyAddWithRemainder(I))
return replaceInstUsesWith(I, V);

if (Value *Res = FoldOrOfSelectSmaxToAbs(I, Builder))
return replaceInstUsesWith(I, Res);

return nullptr;
}

Expand Down
95 changes: 95 additions & 0 deletions llvm/test/Transforms/InstCombine/or.ll
Original file line number Diff line number Diff line change
Expand Up @@ -2113,3 +2113,98 @@ define <4 x i32> @or_zext_nneg_minus_constant_splat(<4 x i8> %a) {
%or = or <4 x i32> %zext, splat (i32 -9)
ret <4 x i32> %or
}

define i8 @or_positive_minus_non_positive_to_abs(i8 %a){
; CHECK-LABEL: @or_positive_minus_non_positive_to_abs(
; CHECK-NEXT: [[TMP2:%.*]] = call i8 @llvm.abs.i8(i8 [[A:%.*]], i1 false)
; CHECK-NEXT: ret i8 [[TMP2]]
;
%b = icmp sgt i8 %a, 0
%mask = sext i1 %b to i8
%neg = sub i8 0, %a
%mask_inv = xor i8 %mask, -1
%c = and i8 %neg, %mask_inv
%d = and i8 %a, %mask
%or = or i8 %c, %d
ret i8 %or
}

; TODO Fold to smax https://alive2.llvm.org/ce/z/wDiDh2
define i8 @or_select_smax_neg_to_abs(i8 %a){
; CHECK-LABEL: @or_select_smax_neg_to_abs(
; CHECK-NEXT: [[SGT0:%.*]] = icmp sgt i8 [[A:%.*]], 0
; CHECK-NEXT: [[NEG:%.*]] = sub nsw i8 0, [[A]]
; CHECK-NEXT: [[OR:%.*]] = select i1 [[SGT0]], i8 0, i8 [[NEG]]
; CHECK-NEXT: ret i8 [[OR]]
;
%sgt0 = icmp sgt i8 %a, 0
%neg = sub nsw i8 0, %a
%sel = select i1 %sgt0, i8 0, i8 %neg
ret i8 %sel
}

; TODO Fold to abs https://alive2.llvm.org/ce/z/DybfHG
define i8 @or_select_smax_smax_to_abs(i8 %a){
; CHECK-LABEL: @or_select_smax_smax_to_abs(
; CHECK-NEXT: [[NEG:%.*]] = sub nsw i8 0, [[A:%.*]]
; CHECK-NEXT: [[SEL:%.*]] = call i8 @llvm.smax.i8(i8 [[NEG]], i8 0)
; CHECK-NEXT: [[MAX:%.*]] = call i8 @llvm.smax.i8(i8 [[A]], i8 0)
; CHECK-NEXT: [[OR:%.*]] = or i8 [[SEL]], [[MAX]]
; CHECK-NEXT: ret i8 [[OR]]
;
%neg = sub nsw i8 0, %a
%sel = call i8 @llvm.smax.i8(i8 %neg, i8 0)
%max = call i8 @llvm.smax.i8(i8 %a, i8 0)
%or = or i8 %sel, %max
ret i8 %or
}

declare i8 @llvm.abs.i8(i8, i1)
declare <2 x i8> @llvm.abs.v2i8(<2 x i8>, i1)

define <2 x i8> @or_sgt_select_smax_to_abs(<2 x i8> %a){
; CHECK-LABEL: @or_sgt_select_smax_to_abs(
; CHECK-NEXT: [[OR:%.*]] = call <2 x i8> @llvm.abs.v2i8(<2 x i8> [[A:%.*]], i1 false)
; CHECK-NEXT: ret <2 x i8> [[OR]]
;
%sgt0 = icmp sgt <2 x i8> %a, zeroinitializer
%neg = sub <2 x i8> zeroinitializer, %a
%sel = select <2 x i1> %sgt0, <2 x i8> zeroinitializer, <2 x i8> %neg
%max = call <2 x i8> @llvm.smax.v2i8(<2 x i8> %a, <2 x i8> zeroinitializer)
%or = or <2 x i8> %sel, %max
ret <2 x i8> %or
}

define <2 x i8> @or_lgt_select_smax_to_abs(<2 x i8> %a){
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
define <2 x i8> @or_lgt_select_smax_to_abs(<2 x i8> %a){
define <2 x i8> @or_slt_select_smax_to_abs(<2 x i8> %a){

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Suggested change
define <2 x i8> @or_lgt_select_smax_to_abs(<2 x i8> %a){
define <2 x i8> @or_slt_select_smax_to_abs(<2 x i8> %a){

done, thanks @dtcxzyw

; CHECK-LABEL: @or_lgt_select_smax_to_abs(
; CHECK-NEXT: [[OR:%.*]] = call <2 x i8> @llvm.abs.v2i8(<2 x i8> [[A:%.*]], i1 false)
; CHECK-NEXT: ret <2 x i8> [[OR]]
;
%slt0 = icmp slt <2 x i8> %a, zeroinitializer
%neg = sub <2 x i8> zeroinitializer, %a
%sel = select <2 x i1> %slt0, <2 x i8> %neg, <2 x i8> zeroinitializer
%max = call <2 x i8> @llvm.smax.v2i8(<2 x i8> %a, <2 x i8> zeroinitializer)
%or = or <2 x i8> %sel, %max
ret <2 x i8> %or
}

; negative test - %d has multiple uses. %or is not folded to abs.

define <2 x i8> @or_select_smax_multi_uses(<2 x i8> %a){
; CHECK-LABEL: @or_select_smax_multi_uses(
; CHECK-NEXT: [[B:%.*]] = icmp sgt <2 x i8> [[A:%.*]], zeroinitializer
; CHECK-NEXT: [[NEG:%.*]] = sub <2 x i8> zeroinitializer, [[A]]
; CHECK-NEXT: [[C:%.*]] = select <2 x i1> [[B]], <2 x i8> zeroinitializer, <2 x i8> [[NEG]]
; CHECK-NEXT: [[D:%.*]] = call <2 x i8> @llvm.smax.v2i8(<2 x i8> [[A]], <2 x i8> zeroinitializer)
; CHECK-NEXT: [[OR1:%.*]] = or <2 x i8> [[C]], [[D]]
; CHECK-NEXT: [[OR:%.*]] = add <2 x i8> [[OR1]], [[D]]
; CHECK-NEXT: ret <2 x i8> [[OR]]
;
%sgt0 = icmp sgt <2 x i8> %a, zeroinitializer
%neg = sub <2 x i8> zeroinitializer, %a
%sel = select <2 x i1> %sgt0, <2 x i8> zeroinitializer, <2 x i8> %neg
%max = call <2 x i8> @llvm.smax.v2i8(<2 x i8> %a, <2 x i8> zeroinitializer)
%or = or <2 x i8> %sel, %max
%add = add <2 x i8> %or, %max
ret <2 x i8> %add
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Missing negative test for the multiple use case

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Missing negative test for the multiple use case

done

Loading