This is an archive of the discontinued LLVM Phabricator instance.

Differential D140602

[RISCV] Add fmin/fmax scalar instructions to isAssociativeAndCommutative
ClosedPublic

Authored by HsiangKai on Dec 22 2022, 7:28 PM.

Details

Reviewers
craig.topper
kito-cheng
asb
luismarques
asi-sc
Commits
rGaf5dd2706c84: [RISCV] Add fmin/fmax scalar instructions to isAssociativeAndCommutative
Summary

Follow-up patch of D140530.

We can add FMIN, FMAX to isAssociativeAndCommutative to
increase instruction-level parallelism by the existing MachineCombiner
pass.

Diff Detail

Event Timeline

HsiangKai created this revision.Dec 22 2022, 7:28 PM
Herald added a project: Restricted Project. · View Herald TranscriptDec 22 2022, 7:28 PM
Herald added subscribers: sunshaoce, VincentWu, StephenFan and 26 others. · View Herald Transcript
HsiangKai requested review of this revision.Dec 22 2022, 7:28 PM
Herald added a project: Restricted Project. · View Herald TranscriptDec 22 2022, 7:28 PM
Herald added subscribers: llvm-commits, • pcwang-thead, eopXD, MaskRay. · View Herald Transcript
Harbormaster completed remote builds in B204716: Diff 485021.Dec 22 2022, 7:29 PM
craig.topper added inline comments.Dec 22 2022, 8:51 PM
llvm/lib/Target/RISCV/RISCVInstrInfo.cpp
1367

I don't think FMIN/FMAX require Reassoc or FmNsz

HsiangKai updated this revision to Diff 485047.Dec 22 2022, 10:41 PM

No need to check RmReassoc and FmNsz for fmin/fmax.

Harbormaster completed remote builds in B204734: Diff 485047.Dec 22 2022, 10:41 PM
asi-sc added inline comments.Dec 23 2022, 2:24 AM
llvm/test/CodeGen/RISCV/machine-combiner.ll
743

I believe we can remove nsz and reassoc flags here as well.

HsiangKai added inline comments.Dec 25 2022, 6:34 PM
llvm/test/CodeGen/RISCV/machine-combiner.ll
743

Thank you. I copied the test cases and forgot to remove them.

HsiangKai updated this revision to Diff 485262.Dec 25 2022, 7:06 PM

Remove unnecessary fast-math flags in the test cases.

Harbormaster completed remote builds in B204896: Diff 485262.Dec 25 2022, 7:06 PM
craig.topper accepted this revision.Dec 25 2022, 10:52 PM

LGTM

This revision is now accepted and ready to land.Dec 25 2022, 10:52 PM
HsiangKai updated this revision to Diff 485281.Dec 26 2022, 3:31 AM

Rebase.

Harbormaster completed remote builds in B204912: Diff 485281.Dec 26 2022, 3:32 AM
asi-sc added a comment.Dec 27 2022, 7:21 AM

LGTM too.

This revision was landed with ongoing or failed builds.Dec 29 2022, 3:59 AM
Closed by commit rGaf5dd2706c84: [RISCV] Add fmin/fmax scalar instructions to isAssociativeAndCommutative (authored by HsiangKai). · Explain Why
This revision was automatically updated to reflect the committed changes.
HsiangKai added a commit: rGaf5dd2706c84: [RISCV] Add fmin/fmax scalar instructions to isAssociativeAndCommutative.

Revision Contents

PathSize
llvm/
lib/
Target/
RISCV/
6 lines
test/
CodeGen/
RISCV/
86 lines

Diff 485603

llvm/lib/Target/RISCV/RISCVInstrInfo.cpp

Show First 20 Lines • Show All 1,358 Lines • ▼ Show 20 Linesbool RISCVInstrInfo::isAssociativeAndCommutative(const MachineInstr &Inst,
unsigned Opc = Inst.getOpcode(); unsigned Opc = Inst.getOpcode();
if (Invert) { if (Invert) {
auto InverseOpcode = getInverseOpcode(Opc); auto InverseOpcode = getInverseOpcode(Opc);
if (!InverseOpcode) if (!InverseOpcode)
return false; return false;
Opc = *InverseOpcode; Opc = *InverseOpcode;
} }
if (isFADD(Opc) || isFMUL(Opc)) if (isFADD(Opc) || isFMUL(Opc))
craig.topperUnsubmitted
Not Done
ReplyInline Actions

I don't think FMIN/FMAX require Reassoc or FmNsz

craig.topper: I don't think FMIN/FMAX require Reassoc or FmNsz
return Inst.getFlag(MachineInstr::MIFlag::FmReassoc) && return Inst.getFlag(MachineInstr::MIFlag::FmReassoc) &&
Inst.getFlag(MachineInstr::MIFlag::FmNsz); Inst.getFlag(MachineInstr::MIFlag::FmNsz);
switch (Opc) { switch (Opc) {
default: default:
return false; return false;
case RISCV::ADD: case RISCV::ADD:
case RISCV::ADDW: case RISCV::ADDW:
Show All 13 Linesbool RISCVInstrInfo::isAssociativeAndCommutative(const MachineInstr &Inst,
// MachineCombiner may reassociate MUL operands and lose the fusion // MachineCombiner may reassociate MUL operands and lose the fusion
// opportunity. // opportunity.
case RISCV::MUL: case RISCV::MUL:
case RISCV::MULW: case RISCV::MULW:
case RISCV::MIN: case RISCV::MIN:
case RISCV::MINU: case RISCV::MINU:
case RISCV::MAX: case RISCV::MAX:
case RISCV::MAXU: case RISCV::MAXU:
case RISCV::FMIN_H:
case RISCV::FMIN_S:
case RISCV::FMIN_D:
case RISCV::FMAX_H:
case RISCV::FMAX_S:
case RISCV::FMAX_D:
return true; return true;
} }
return false; return false;
} }
std::optional<unsigned> std::optional<unsigned>
RISCVInstrInfo::getInverseOpcode(unsigned Opcode) const { RISCVInstrInfo::getInverseOpcode(unsigned Opcode) const {
▲ Show 20 Lines • Show All 1,463 Lines • Show Last 20 Lines

llvm/test/CodeGen/RISCV/machine-combiner.ll

; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py ; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
; RUN: llc -mtriple=riscv64 -mattr=+d,+zbb -verify-machineinstrs -mcpu=sifive-u74 \ ; RUN: llc -mtriple=riscv64 -mattr=+d,+zbb,+zfh -verify-machineinstrs -mcpu=sifive-u74 \
; RUN: -O1 -riscv-enable-machine-combiner=true < %s | \ ; RUN: -O1 -riscv-enable-machine-combiner=true < %s | \
; RUN: FileCheck %s ; RUN: FileCheck %s
define double @test_reassoc_fadd1(double %a0, double %a1, double %a2, double %a3) { define double @test_reassoc_fadd1(double %a0, double %a1, double %a2, double %a3) {
; CHECK-LABEL: test_reassoc_fadd1: ; CHECK-LABEL: test_reassoc_fadd1:
; CHECK: # %bb.0: ; CHECK: # %bb.0:
; CHECK-NEXT: fadd.d ft0, fa0, fa1 ; CHECK-NEXT: fadd.d ft0, fa0, fa1
; CHECK-NEXT: fadd.d ft1, fa2, fa3 ; CHECK-NEXT: fadd.d ft1, fa2, fa3
▲ Show 20 Lines • Show All 724 Lines • ▼ Show 20 Lines
define i8 @test_reassoc_minu_i8(i8 %a0, i8 %a1, i8 %a2, i8 %a3) { define i8 @test_reassoc_minu_i8(i8 %a0, i8 %a1, i8 %a2, i8 %a3) {
; CHECK-LABEL: test_reassoc_minu_i8: ; CHECK-LABEL: test_reassoc_minu_i8:
; CHECK: # %bb.0: ; CHECK: # %bb.0:
; CHECK-NEXT: andi a3, a3, 255 ; CHECK-NEXT: andi a3, a3, 255
; CHECK-NEXT: andi a2, a2, 255 ; CHECK-NEXT: andi a2, a2, 255
; CHECK-NEXT: andi a1, a1, 255 ; CHECK-NEXT: andi a1, a1, 255
; CHECK-NEXT: andi a0, a0, 255 ; CHECK-NEXT: andi a0, a0, 255
; CHECK-NEXT: minu a0, a0, a1 ; CHECK-NEXT: minu a0, a0, a1
; CHECK-NEXT: minu a1, a2, a3 ; CHECK-NEXT: minu a1, a2, a3
asi-scUnsubmitted
Not Done
ReplyInline Actions

I believe we can remove nsz and reassoc flags here as well.

asi-sc: I believe we can remove nsz and reassoc flags here as well.
HsiangKaiAuthorUnsubmitted
Done
ReplyInline Actions

Thank you. I copied the test cases and forgot to remove them.

HsiangKai: Thank you. I copied the test cases and forgot to remove them.
; CHECK-NEXT: minu a0, a0, a1 ; CHECK-NEXT: minu a0, a0, a1
; CHECK-NEXT: ret ; CHECK-NEXT: ret
%t0 = call i8 @llvm.umin.i8(i8 %a0, i8 %a1) %t0 = call i8 @llvm.umin.i8(i8 %a0, i8 %a1)
%t1 = call i8 @llvm.umin.i8(i8 %t0, i8 %a2) %t1 = call i8 @llvm.umin.i8(i8 %t0, i8 %a2)
%t2 = call i8 @llvm.umin.i8(i8 %t1, i8 %a3) %t2 = call i8 @llvm.umin.i8(i8 %t1, i8 %a3)
ret i8 %t2 ret i8 %t2
} }
▲ Show 20 Lines • Show All 231 Lines • ▼ Show 20 Lines
; CHECK-NEXT: max a0, a0, a1 ; CHECK-NEXT: max a0, a0, a1
; CHECK-NEXT: ret ; CHECK-NEXT: ret
%t0 = call i64 @llvm.smax.i64(i64 %a0, i64 %a1) %t0 = call i64 @llvm.smax.i64(i64 %a0, i64 %a1)
%t1 = call i64 @llvm.smax.i64(i64 %t0, i64 %a2) %t1 = call i64 @llvm.smax.i64(i64 %t0, i64 %a2)
%t2 = call i64 @llvm.smax.i64(i64 %t1, i64 %a3) %t2 = call i64 @llvm.smax.i64(i64 %t1, i64 %a3)
ret i64 %t2 ret i64 %t2
} }
define half @test_fmin_f16(half %a0, half %a1, half %a2, half %a3) {
; CHECK-LABEL: test_fmin_f16:
; CHECK: # %bb.0:
; CHECK-NEXT: fmin.h ft0, fa0, fa1
; CHECK-NEXT: fmin.h ft1, fa2, fa3
; CHECK-NEXT: fmin.h fa0, ft0, ft1
; CHECK-NEXT: ret
%t0 = call half @llvm.minnum.f16(half %a0, half %a1)
%t1 = call half @llvm.minnum.f16(half %t0, half %a2)
%t2 = call half @llvm.minnum.f16(half %t1, half %a3)
ret half %t2
}
define float @test_fmin_f32(float %a0, float %a1, float %a2, float %a3) {
; CHECK-LABEL: test_fmin_f32:
; CHECK: # %bb.0:
; CHECK-NEXT: fmin.s ft0, fa0, fa1
; CHECK-NEXT: fmin.s ft1, fa2, fa3
; CHECK-NEXT: fmin.s fa0, ft0, ft1
; CHECK-NEXT: ret
%t0 = call float @llvm.minnum.f32(float %a0, float %a1)
%t1 = call float @llvm.minnum.f32(float %t0, float %a2)
%t2 = call float @llvm.minnum.f32(float %t1, float %a3)
ret float %t2
}
define double @test_fmin_f64(double %a0, double %a1, double %a2, double %a3) {
; CHECK-LABEL: test_fmin_f64:
; CHECK: # %bb.0:
; CHECK-NEXT: fmin.d ft0, fa0, fa1
; CHECK-NEXT: fmin.d ft1, fa2, fa3
; CHECK-NEXT: fmin.d fa0, ft0, ft1
; CHECK-NEXT: ret
%t0 = call double @llvm.minnum.f64(double %a0, double %a1)
%t1 = call double @llvm.minnum.f64(double %t0, double %a2)
%t2 = call double @llvm.minnum.f64(double %t1, double %a3)
ret double %t2
}
define half @test_fmax_f16(half %a0, half %a1, half %a2, half %a3) {
; CHECK-LABEL: test_fmax_f16:
; CHECK: # %bb.0:
; CHECK-NEXT: fmax.h ft0, fa0, fa1
; CHECK-NEXT: fmax.h ft1, fa2, fa3
; CHECK-NEXT: fmax.h fa0, ft0, ft1
; CHECK-NEXT: ret
%t0 = call half @llvm.maxnum.f16(half %a0, half %a1)
%t1 = call half @llvm.maxnum.f16(half %t0, half %a2)
%t2 = call half @llvm.maxnum.f16(half %t1, half %a3)
ret half %t2
}
define float @test_fmax_f32(float %a0, float %a1, float %a2, float %a3) {
; CHECK-LABEL: test_fmax_f32:
; CHECK: # %bb.0:
; CHECK-NEXT: fmax.s ft0, fa0, fa1
; CHECK-NEXT: fmax.s ft1, fa2, fa3
; CHECK-NEXT: fmax.s fa0, ft0, ft1
; CHECK-NEXT: ret
%t0 = call float @llvm.maxnum.f32(float %a0, float %a1)
%t1 = call float @llvm.maxnum.f32(float %t0, float %a2)
%t2 = call float @llvm.maxnum.f32(float %t1, float %a3)
ret float %t2
}
define double @test_fmax_f64(double %a0, double %a1, double %a2, double %a3) {
; CHECK-LABEL: test_fmax_f64:
; CHECK: # %bb.0:
; CHECK-NEXT: fmax.d ft0, fa0, fa1
; CHECK-NEXT: fmax.d ft1, fa2, fa3
; CHECK-NEXT: fmax.d fa0, ft0, ft1
; CHECK-NEXT: ret
%t0 = call double @llvm.maxnum.f64(double %a0, double %a1)
%t1 = call double @llvm.maxnum.f64(double %t0, double %a2)
%t2 = call double @llvm.maxnum.f64(double %t1, double %a3)
ret double %t2
}
declare i8 @llvm.umin.i8(i8 %a, i8 %b) declare i8 @llvm.umin.i8(i8 %a, i8 %b)
declare i16 @llvm.umin.i16(i16 %a, i16 %b) declare i16 @llvm.umin.i16(i16 %a, i16 %b)
declare i32 @llvm.umin.i32(i32 %a, i32 %b) declare i32 @llvm.umin.i32(i32 %a, i32 %b)
declare i64 @llvm.umin.i64(i64 %a, i64 %b) declare i64 @llvm.umin.i64(i64 %a, i64 %b)
declare i8 @llvm.smin.i8(i8 %a, i8 %b) declare i8 @llvm.smin.i8(i8 %a, i8 %b)
declare i16 @llvm.smin.i16(i16 %a, i16 %b) declare i16 @llvm.smin.i16(i16 %a, i16 %b)
declare i32 @llvm.smin.i32(i32 %a, i32 %b) declare i32 @llvm.smin.i32(i32 %a, i32 %b)
declare i64 @llvm.smin.i64(i64 %a, i64 %b) declare i64 @llvm.smin.i64(i64 %a, i64 %b)
declare i8 @llvm.umax.i8(i8 %a, i8 %b) declare i8 @llvm.umax.i8(i8 %a, i8 %b)
declare i16 @llvm.umax.i16(i16 %a, i16 %b) declare i16 @llvm.umax.i16(i16 %a, i16 %b)
declare i32 @llvm.umax.i32(i32 %a, i32 %b) declare i32 @llvm.umax.i32(i32 %a, i32 %b)
declare i64 @llvm.umax.i64(i64 %a, i64 %b) declare i64 @llvm.umax.i64(i64 %a, i64 %b)
declare i8 @llvm.smax.i8(i8 %a, i8 %b) declare i8 @llvm.smax.i8(i8 %a, i8 %b)
declare i16 @llvm.smax.i16(i16 %a, i16 %b) declare i16 @llvm.smax.i16(i16 %a, i16 %b)
declare i32 @llvm.smax.i32(i32 %a, i32 %b) declare i32 @llvm.smax.i32(i32 %a, i32 %b)
declare i64 @llvm.smax.i64(i64 %a, i64 %b) declare i64 @llvm.smax.i64(i64 %a, i64 %b)
declare half @llvm.minnum.f16(half, half)
declare float @llvm.minnum.f32(float, float)
declare double @llvm.minnum.f64(double, double)
declare half @llvm.maxnum.f16(half, half)
declare float @llvm.maxnum.f32(float, float)
declare double @llvm.maxnum.f64(double, double)