[llvm] r334520 - [MIR][MachineCSE] Implementing proper MachineInstr::getNumExplicitDefs()
Roman Tereshin via llvm-commits
llvm-commits at lists.llvm.org
Tue Jun 12 11:30:37 PDT 2018
Author: rtereshin
Date: Tue Jun 12 11:30:37 2018
New Revision: 334520
URL: http://llvm.org/viewvc/llvm-project?rev=334520&view=rev
Log:
[MIR][MachineCSE] Implementing proper MachineInstr::getNumExplicitDefs()
Apparently, MachineInstr class definition as well as pretty much all of
the machine passes assume that the only kind of MachineInstr's operands
that is variadic for variadic opcodes is explicit non-definitions.
In particular, this assumption is made by MachineInstr::defs(), uses(),
and explicit_uses() methods, as well as by MachineCSE pass.
The assumption is incorrect judging from at least TableGen backend
implementation, that recognizes variable_ops in OutOperandList, and the
very existence of G_UNMERGE_VALUES generic opcode, or ARM load multiple
instructions, all of which have variadic defs.
In particular, MachineCSE pass breaks MIR with CSE'able G_UNMERGE_VALUES
instructions in it.
This commit implements MachineInstr::getNumExplicitDefs() similar to
pre-existing MachineInstr::getNumExplicitOperands(), fixes
MachineInstr::defs(), uses(), and explicit_uses(), and fixes MachineCSE
pass.
As the issue addressed seems to affect only machine passes that could be
ran mid-GlobalISel pipeline at the moment, the other passes aren't fixed
by this commit, like MachineLICM: that could be done on per-pass basis
when (if ever) they get adopted for GlobalISel.
Reviewed By: arsenm
Differential Revision: https://reviews.llvm.org/D45640
Modified:
llvm/trunk/include/llvm/CodeGen/MachineInstr.h
llvm/trunk/lib/CodeGen/MachineCSE.cpp
llvm/trunk/lib/CodeGen/MachineInstr.cpp
llvm/trunk/test/CodeGen/AArch64/GlobalISel/machine-cse-mid-pipeline.mir
Modified: llvm/trunk/include/llvm/CodeGen/MachineInstr.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/CodeGen/MachineInstr.h?rev=334520&r1=334519&r2=334520&view=diff
==============================================================================
--- llvm/trunk/include/llvm/CodeGen/MachineInstr.h (original)
+++ llvm/trunk/include/llvm/CodeGen/MachineInstr.h Tue Jun 12 11:30:37 2018
@@ -322,6 +322,11 @@ public:
return Operands[i];
}
+ /// Returns the total number of definitions.
+ unsigned getNumDefs() const {
+ return getNumExplicitDefs() + MCID->getNumImplicitDefs();
+ }
+
/// Return true if operand \p OpIdx is a subregister index.
bool isOperandSubregIdx(unsigned OpIdx) const {
assert(getOperand(OpIdx).getType() == MachineOperand::MO_Immediate &&
@@ -340,6 +345,9 @@ public:
/// Returns the number of non-implicit operands.
unsigned getNumExplicitOperands() const;
+ /// Returns the number of non-implicit definitions.
+ unsigned getNumExplicitDefs() const;
+
/// iterator/begin/end - Iterate over all operands of a machine instruction.
using mop_iterator = MachineOperand *;
using const_mop_iterator = const MachineOperand *;
@@ -374,31 +382,29 @@ public:
/// Implicit definition are not included!
iterator_range<mop_iterator> defs() {
return make_range(operands_begin(),
- operands_begin() + getDesc().getNumDefs());
+ operands_begin() + getNumExplicitDefs());
}
/// \copydoc defs()
iterator_range<const_mop_iterator> defs() const {
return make_range(operands_begin(),
- operands_begin() + getDesc().getNumDefs());
+ operands_begin() + getNumExplicitDefs());
}
/// Returns a range that includes all operands that are register uses.
/// This may include unrelated operands which are not register uses.
iterator_range<mop_iterator> uses() {
- return make_range(operands_begin() + getDesc().getNumDefs(),
- operands_end());
+ return make_range(operands_begin() + getNumExplicitDefs(), operands_end());
}
/// \copydoc uses()
iterator_range<const_mop_iterator> uses() const {
- return make_range(operands_begin() + getDesc().getNumDefs(),
- operands_end());
+ return make_range(operands_begin() + getNumExplicitDefs(), operands_end());
}
iterator_range<mop_iterator> explicit_uses() {
- return make_range(operands_begin() + getDesc().getNumDefs(),
- operands_begin() + getNumExplicitOperands() );
+ return make_range(operands_begin() + getNumExplicitDefs(),
+ operands_begin() + getNumExplicitOperands());
}
iterator_range<const_mop_iterator> explicit_uses() const {
- return make_range(operands_begin() + getDesc().getNumDefs(),
- operands_begin() + getNumExplicitOperands() );
+ return make_range(operands_begin() + getNumExplicitDefs(),
+ operands_begin() + getNumExplicitOperands());
}
/// Returns the number of the operand iterator \p I points to.
Modified: llvm/trunk/lib/CodeGen/MachineCSE.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/MachineCSE.cpp?rev=334520&r1=334519&r2=334520&view=diff
==============================================================================
--- llvm/trunk/lib/CodeGen/MachineCSE.cpp (original)
+++ llvm/trunk/lib/CodeGen/MachineCSE.cpp Tue Jun 12 11:30:37 2018
@@ -550,8 +550,7 @@ bool MachineCSE::ProcessBlock(MachineBas
// Check if it's profitable to perform this CSE.
bool DoCSE = true;
- unsigned NumDefs = MI->getDesc().getNumDefs() +
- MI->getDesc().getNumImplicitDefs();
+ unsigned NumDefs = MI->getNumDefs();
for (unsigned i = 0, e = MI->getNumOperands(); NumDefs && i != e; ++i) {
MachineOperand &MO = MI->getOperand(i);
Modified: llvm/trunk/lib/CodeGen/MachineInstr.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/MachineInstr.cpp?rev=334520&r1=334519&r2=334520&view=diff
==============================================================================
--- llvm/trunk/lib/CodeGen/MachineInstr.cpp (original)
+++ llvm/trunk/lib/CodeGen/MachineInstr.cpp Tue Jun 12 11:30:37 2018
@@ -519,21 +519,39 @@ void MachineInstr::eraseFromBundle() {
getParent()->erase_instr(this);
}
-/// getNumExplicitOperands - Returns the number of non-implicit operands.
-///
unsigned MachineInstr::getNumExplicitOperands() const {
unsigned NumOperands = MCID->getNumOperands();
if (!MCID->isVariadic())
return NumOperands;
- for (unsigned i = NumOperands, e = getNumOperands(); i != e; ++i) {
- const MachineOperand &MO = getOperand(i);
- if (!MO.isReg() || !MO.isImplicit())
- NumOperands++;
+ for (unsigned I = NumOperands, E = getNumOperands(); I != E; ++I) {
+ const MachineOperand &MO = getOperand(I);
+ // The operands must always be in the following order:
+ // - explicit reg defs,
+ // - other explicit operands (reg uses, immediates, etc.),
+ // - implicit reg defs
+ // - implicit reg uses
+ if (MO.isReg() && MO.isImplicit())
+ break;
+ ++NumOperands;
}
return NumOperands;
}
+unsigned MachineInstr::getNumExplicitDefs() const {
+ unsigned NumDefs = MCID->getNumDefs();
+ if (!MCID->isVariadic())
+ return NumDefs;
+
+ for (unsigned I = NumDefs, E = getNumOperands(); I != E; ++I) {
+ const MachineOperand &MO = getOperand(I);
+ if (!MO.isReg() || !MO.isDef() || MO.isImplicit())
+ break;
+ ++NumDefs;
+ }
+ return NumDefs;
+}
+
void MachineInstr::bundleWithPred() {
assert(!isBundledWithPred() && "MI is already bundled with its predecessor");
setFlag(BundledPred);
Modified: llvm/trunk/test/CodeGen/AArch64/GlobalISel/machine-cse-mid-pipeline.mir
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/AArch64/GlobalISel/machine-cse-mid-pipeline.mir?rev=334520&r1=334519&r2=334520&view=diff
==============================================================================
--- llvm/trunk/test/CodeGen/AArch64/GlobalISel/machine-cse-mid-pipeline.mir (original)
+++ llvm/trunk/test/CodeGen/AArch64/GlobalISel/machine-cse-mid-pipeline.mir Tue Jun 12 11:30:37 2018
@@ -179,3 +179,102 @@ body: |
$w0 = COPY %4
RET_ReallyLR implicit $w0
...
+---
+name: variadic_defs_unmerge_vector
+legalized: true
+regBankSelected: false
+selected: false
+body: |
+ ; CHECK-LABEL: name: variadic_defs_unmerge_vector
+ ; CHECK: [[COPY:%[0-9]+]]:_(<4 x s16>) = COPY $d0
+ ; CHECK-NEXT: [[UV0:%[0-9]+]]:_(s16), [[UV1:%[0-9]+]]:_(s16), [[UV2:%[0-9]+]]:_(s16), [[UV3:%[0-9]+]]:_(s16) = G_UNMERGE_VALUES [[COPY]](<4 x s16>)
+ ; CHECK-NEXT: [[ANYEXT0:%[0-9]+]]:_(s32) = G_ANYEXT [[UV0]](s16)
+ ; CHECK-NEXT: [[ANYEXT1:%[0-9]+]]:_(s32) = G_ANYEXT [[UV1]](s16)
+ ; CHECK-NEXT: [[ANYEXT2:%[0-9]+]]:_(s32) = G_ANYEXT [[UV2]](s16)
+ ; CHECK-NEXT: [[ANYEXT3:%[0-9]+]]:_(s32) = G_ANYEXT [[UV3]](s16)
+ ; CHECK-NEXT: [[ADD0:%[0-9]+]]:_(s32) = G_ADD [[ANYEXT0]], [[ANYEXT1]]
+ ; CHECK-NEXT: [[ADD1:%[0-9]+]]:_(s32) = G_ADD [[ANYEXT2]], [[ANYEXT3]]
+ ; CHECK-NEXT: [[ADD2:%[0-9]+]]:_(s32) = G_ADD [[ADD0]], [[ADD1]]
+ ; CHECK-NEXT: $w0 = COPY [[ADD2]](s32)
+ ; CHECK-NEXT: RET_ReallyLR implicit $w0
+ bb.0:
+ %0 :_(<4 x s16>) = COPY $d0
+ %1 :_(s16), %2 :_(s16), %3 :_(s16), %4 :_(s16) = G_UNMERGE_VALUES %0(<4 x s16>)
+ %5 :_(s16), %6 :_(s16), %7 :_(s16), %8 :_(s16) = G_UNMERGE_VALUES %0(<4 x s16>)
+ %9 :_(s16), %10:_(s16), %11:_(s16), %12:_(s16) = G_UNMERGE_VALUES %0(<4 x s16>)
+ %13:_(s16), %14:_(s16), %15:_(s16), %16:_(s16) = G_UNMERGE_VALUES %0(<4 x s16>)
+ %17:_(s32) = G_ANYEXT %1 (s16)
+ %18:_(s32) = G_ANYEXT %6 (s16)
+ %19:_(s32) = G_ANYEXT %11(s16)
+ %20:_(s32) = G_ANYEXT %16(s16)
+ %21:_(s32) = G_ADD %17, %18
+ %22:_(s32) = G_ADD %19, %20
+ %23:_(s32) = G_ADD %21, %22
+ $w0 = COPY %23(s32)
+ RET_ReallyLR implicit $w0
+...
+---
+name: variadic_defs_unmerge_scalar
+legalized: true
+regBankSelected: false
+selected: false
+body: |
+ ; CHECK-LABEL: name: variadic_defs_unmerge_scalar
+ ; CHECK: [[COPY:%[0-9]+]]:_(s64) = COPY $d0
+ ; CHECK-NEXT: [[UV0:%[0-9]+]]:_(s16), [[UV1:%[0-9]+]]:_(s16), [[UV2:%[0-9]+]]:_(s16), [[UV3:%[0-9]+]]:_(s16) = G_UNMERGE_VALUES [[COPY]](s64)
+ ; CHECK-NEXT: [[ANYEXT0:%[0-9]+]]:_(s32) = G_ANYEXT [[UV0]](s16)
+ ; CHECK-NEXT: [[ANYEXT1:%[0-9]+]]:_(s32) = G_ANYEXT [[UV1]](s16)
+ ; CHECK-NEXT: [[ANYEXT2:%[0-9]+]]:_(s32) = G_ANYEXT [[UV2]](s16)
+ ; CHECK-NEXT: [[ANYEXT3:%[0-9]+]]:_(s32) = G_ANYEXT [[UV3]](s16)
+ ; CHECK-NEXT: [[ADD0:%[0-9]+]]:_(s32) = G_ADD [[ANYEXT0]], [[ANYEXT1]]
+ ; CHECK-NEXT: [[ADD1:%[0-9]+]]:_(s32) = G_ADD [[ANYEXT2]], [[ANYEXT3]]
+ ; CHECK-NEXT: [[ADD2:%[0-9]+]]:_(s32) = G_ADD [[ADD0]], [[ADD1]]
+ ; CHECK-NEXT: $w0 = COPY [[ADD2]](s32)
+ ; CHECK-NEXT: RET_ReallyLR implicit $w0
+ bb.0:
+ %0 :_(s64) = COPY $d0
+ %1 :_(s16), %2 :_(s16), %3 :_(s16), %4 :_(s16) = G_UNMERGE_VALUES %0(s64)
+ %5 :_(s16), %6 :_(s16), %7 :_(s16), %8 :_(s16) = G_UNMERGE_VALUES %0(s64)
+ %9 :_(s16), %10:_(s16), %11:_(s16), %12:_(s16) = G_UNMERGE_VALUES %0(s64)
+ %13:_(s16), %14:_(s16), %15:_(s16), %16:_(s16) = G_UNMERGE_VALUES %0(s64)
+ %17:_(s32) = G_ANYEXT %1 (s16)
+ %18:_(s32) = G_ANYEXT %6 (s16)
+ %19:_(s32) = G_ANYEXT %11(s16)
+ %20:_(s32) = G_ANYEXT %16(s16)
+ %21:_(s32) = G_ADD %17, %18
+ %22:_(s32) = G_ADD %19, %20
+ %23:_(s32) = G_ADD %21, %22
+ $w0 = COPY %23(s32)
+ RET_ReallyLR implicit $w0
+...
+---
+name: variadic_defs_unmerge_scalar_asym
+legalized: true
+regBankSelected: false
+selected: false
+body: |
+ ; CHECK-LABEL: name: variadic_defs_unmerge_scalar_asym
+ ; CHECK: [[COPY:%[0-9]+]]:_(s64) = COPY $d0
+ ; CHECK-NEXT: [[UV0:%[0-9]+]]:_(s16), [[UV1:%[0-9]+]]:_(s16), [[UV2:%[0-9]+]]:_(s16), [[UV3:%[0-9]+]]:_(s16) = G_UNMERGE_VALUES [[COPY]](s64)
+ ; CHECK-NEXT: [[UV01:%[0-9]+]]:_(s32), [[UV23:%[0-9]+]]:_(s32) = G_UNMERGE_VALUES [[COPY]](s64)
+ ; CHECK-NEXT: [[ANYEXT0:%[0-9]+]]:_(s32) = G_ANYEXT [[UV0]](s16)
+ ; CHECK-NEXT: [[ANYEXT1:%[0-9]+]]:_(s32) = G_ANYEXT [[UV1]](s16)
+ ; CHECK-NEXT: [[ADD0:%[0-9]+]]:_(s32) = G_ADD [[ANYEXT0]], [[ANYEXT1]]
+ ; CHECK-NEXT: [[ADD1:%[0-9]+]]:_(s32) = G_ADD [[UV01]], [[UV23]]
+ ; CHECK-NEXT: [[ADD2:%[0-9]+]]:_(s32) = G_ADD [[ADD0]], [[ADD1]]
+ ; CHECK-NEXT: $w0 = COPY [[ADD2]](s32)
+ ; CHECK-NEXT: RET_ReallyLR implicit $w0
+ bb.0:
+ %0 :_(s64) = COPY $d0
+ %1 :_(s16), %2 :_(s16), %3 :_(s16), %4 :_(s16) = G_UNMERGE_VALUES %0(s64)
+ %9 :_(s32), %10:_(s32) = G_UNMERGE_VALUES %0(s64)
+ %5 :_(s16), %6 :_(s16), %7 :_(s16), %8 :_(s16) = G_UNMERGE_VALUES %0(s64)
+ %11:_(s32), %12:_(s32) = G_UNMERGE_VALUES %0(s64)
+ %17:_(s32) = G_ANYEXT %1 (s16)
+ %18:_(s32) = G_ANYEXT %6 (s16)
+ %21:_(s32) = G_ADD %17, %18
+ %22:_(s32) = G_ADD %9, %12
+ %23:_(s32) = G_ADD %21, %22
+ $w0 = COPY %23(s32)
+ RET_ReallyLR implicit $w0
+...
More information about the llvm-commits
mailing list