[llvm] r304442 - [Hexagon] Fix dependence check in the packetizer
Krzysztof Parzyszek via llvm-commits
llvm-commits at lists.llvm.org
Thu Jun 1 11:02:41 PDT 2017
Author: kparzysz
Date: Thu Jun 1 13:02:40 2017
New Revision: 304442
URL: http://llvm.org/viewvc/llvm-project?rev=304442&view=rev
Log:
[Hexagon] Fix dependence check in the packetizer
An incorrect check in the packetizer lead to an attempt to convert
an unconditional branch to a .new (conditional) form.
Added:
llvm/trunk/test/CodeGen/Hexagon/invalid-dotnew-attempt.mir
Modified:
llvm/trunk/lib/Target/Hexagon/HexagonInstrInfo.cpp
llvm/trunk/lib/Target/Hexagon/HexagonInstrInfo.h
llvm/trunk/lib/Target/Hexagon/HexagonVLIWPacketizer.cpp
Modified: llvm/trunk/lib/Target/Hexagon/HexagonInstrInfo.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/Hexagon/HexagonInstrInfo.cpp?rev=304442&r1=304441&r2=304442&view=diff
==============================================================================
--- llvm/trunk/lib/Target/Hexagon/HexagonInstrInfo.cpp (original)
+++ llvm/trunk/lib/Target/Hexagon/HexagonInstrInfo.cpp Thu Jun 1 13:02:40 2017
@@ -1769,161 +1769,6 @@ bool HexagonInstrInfo::isCompoundBranchI
return getType(MI) == HexagonII::TypeCJ && MI.isBranch();
}
-bool HexagonInstrInfo::isCondInst(const MachineInstr &MI) const {
- return (MI.isBranch() && isPredicated(MI)) ||
- isConditionalTransfer(MI) ||
- isConditionalALU32(MI) ||
- isConditionalLoad(MI) ||
- // Predicated stores which don't have a .new on any operands.
- (MI.mayStore() && isPredicated(MI) && !isNewValueStore(MI) &&
- !isPredicatedNew(MI));
-}
-
-bool HexagonInstrInfo::isConditionalALU32(const MachineInstr &MI) const {
- switch (MI.getOpcode()) {
- case Hexagon::A2_paddf:
- case Hexagon::A2_paddfnew:
- case Hexagon::A2_paddif:
- case Hexagon::A2_paddifnew:
- case Hexagon::A2_paddit:
- case Hexagon::A2_padditnew:
- case Hexagon::A2_paddt:
- case Hexagon::A2_paddtnew:
- case Hexagon::A2_pandf:
- case Hexagon::A2_pandfnew:
- case Hexagon::A2_pandt:
- case Hexagon::A2_pandtnew:
- case Hexagon::A2_porf:
- case Hexagon::A2_porfnew:
- case Hexagon::A2_port:
- case Hexagon::A2_portnew:
- case Hexagon::A2_psubf:
- case Hexagon::A2_psubfnew:
- case Hexagon::A2_psubt:
- case Hexagon::A2_psubtnew:
- case Hexagon::A2_pxorf:
- case Hexagon::A2_pxorfnew:
- case Hexagon::A2_pxort:
- case Hexagon::A2_pxortnew:
- case Hexagon::A4_paslhf:
- case Hexagon::A4_paslhfnew:
- case Hexagon::A4_paslht:
- case Hexagon::A4_paslhtnew:
- case Hexagon::A4_pasrhf:
- case Hexagon::A4_pasrhfnew:
- case Hexagon::A4_pasrht:
- case Hexagon::A4_pasrhtnew:
- case Hexagon::A4_psxtbf:
- case Hexagon::A4_psxtbfnew:
- case Hexagon::A4_psxtbt:
- case Hexagon::A4_psxtbtnew:
- case Hexagon::A4_psxthf:
- case Hexagon::A4_psxthfnew:
- case Hexagon::A4_psxtht:
- case Hexagon::A4_psxthtnew:
- case Hexagon::A4_pzxtbf:
- case Hexagon::A4_pzxtbfnew:
- case Hexagon::A4_pzxtbt:
- case Hexagon::A4_pzxtbtnew:
- case Hexagon::A4_pzxthf:
- case Hexagon::A4_pzxthfnew:
- case Hexagon::A4_pzxtht:
- case Hexagon::A4_pzxthtnew:
- case Hexagon::C2_ccombinewf:
- case Hexagon::C2_ccombinewt:
- return true;
- }
- return false;
-}
-
-// FIXME - Function name and it's functionality don't match.
-// It should be renamed to hasPredNewOpcode()
-bool HexagonInstrInfo::isConditionalLoad(const MachineInstr &MI) const {
- if (!MI.getDesc().mayLoad() || !isPredicated(MI))
- return false;
-
- int PNewOpcode = Hexagon::getPredNewOpcode(MI.getOpcode());
- // Instruction with valid predicated-new opcode can be promoted to .new.
- return PNewOpcode >= 0;
-}
-
-// Returns true if an instruction is a conditional store.
-//
-// Note: It doesn't include conditional new-value stores as they can't be
-// converted to .new predicate.
-bool HexagonInstrInfo::isConditionalStore(const MachineInstr &MI) const {
- switch (MI.getOpcode()) {
- default: return false;
- case Hexagon::S4_storeirbt_io:
- case Hexagon::S4_storeirbf_io:
- case Hexagon::S4_pstorerbt_rr:
- case Hexagon::S4_pstorerbf_rr:
- case Hexagon::S2_pstorerbt_io:
- case Hexagon::S2_pstorerbf_io:
- case Hexagon::S2_pstorerbt_pi:
- case Hexagon::S2_pstorerbf_pi:
- case Hexagon::S2_pstorerdt_io:
- case Hexagon::S2_pstorerdf_io:
- case Hexagon::S4_pstorerdt_rr:
- case Hexagon::S4_pstorerdf_rr:
- case Hexagon::S2_pstorerdt_pi:
- case Hexagon::S2_pstorerdf_pi:
- case Hexagon::S2_pstorerht_io:
- case Hexagon::S2_pstorerhf_io:
- case Hexagon::S4_storeirht_io:
- case Hexagon::S4_storeirhf_io:
- case Hexagon::S4_pstorerht_rr:
- case Hexagon::S4_pstorerhf_rr:
- case Hexagon::S2_pstorerht_pi:
- case Hexagon::S2_pstorerhf_pi:
- case Hexagon::S2_pstorerit_io:
- case Hexagon::S2_pstorerif_io:
- case Hexagon::S4_storeirit_io:
- case Hexagon::S4_storeirif_io:
- case Hexagon::S4_pstorerit_rr:
- case Hexagon::S4_pstorerif_rr:
- case Hexagon::S2_pstorerit_pi:
- case Hexagon::S2_pstorerif_pi:
-
- // V4 global address store before promoting to dot new.
- case Hexagon::S4_pstorerdt_abs:
- case Hexagon::S4_pstorerdf_abs:
- case Hexagon::S4_pstorerbt_abs:
- case Hexagon::S4_pstorerbf_abs:
- case Hexagon::S4_pstorerht_abs:
- case Hexagon::S4_pstorerhf_abs:
- case Hexagon::S4_pstorerit_abs:
- case Hexagon::S4_pstorerif_abs:
- return true;
-
- // Predicated new value stores (i.e. if (p0) memw(..)=r0.new) are excluded
- // from the "Conditional Store" list. Because a predicated new value store
- // would NOT be promoted to a double dot new store.
- // This function returns yes for those stores that are predicated but not
- // yet promoted to predicate dot new instructions.
- }
-}
-
-bool HexagonInstrInfo::isConditionalTransfer(const MachineInstr &MI) const {
- switch (MI.getOpcode()) {
- case Hexagon::A2_tfrt:
- case Hexagon::A2_tfrf:
- case Hexagon::C2_cmoveit:
- case Hexagon::C2_cmoveif:
- case Hexagon::A2_tfrtnew:
- case Hexagon::A2_tfrfnew:
- case Hexagon::C2_cmovenewit:
- case Hexagon::C2_cmovenewif:
- case Hexagon::A2_tfrpt:
- case Hexagon::A2_tfrpf:
- return true;
-
- default:
- return false;
- }
- return false;
-}
-
// TODO: In order to have isExtendable for fpimm/f32Ext, we need to handle
// isFPImm and later getFPImm as well.
bool HexagonInstrInfo::isConstExtended(const MachineInstr &MI) const {
@@ -3474,6 +3319,8 @@ int HexagonInstrInfo::getDotNewOp(const
// Returns the opcode to use when converting MI, which is a conditional jump,
// into a conditional instruction which uses the .new value of the predicate.
// We also use branch probabilities to add a hint to the jump.
+// If MBPI is null, all edges will be treated as equally likely for the
+// purposes of establishing a predication hint.
int HexagonInstrInfo::getDotNewPredJumpOp(const MachineInstr &MI,
const MachineBranchProbabilityInfo *MBPI) const {
// We assume that block can have at most two successors.
@@ -3482,9 +3329,16 @@ int HexagonInstrInfo::getDotNewPredJumpO
bool Taken = false;
const BranchProbability OneHalf(1, 2);
+ auto getEdgeProbability = [MBPI] (const MachineBasicBlock *Src,
+ const MachineBasicBlock *Dst) {
+ if (MBPI)
+ return MBPI->getEdgeProbability(Src, Dst);
+ return BranchProbability(1, Src->succ_size());
+ };
+
if (BrTarget.isMBB()) {
const MachineBasicBlock *Dst = BrTarget.getMBB();
- Taken = MBPI->getEdgeProbability(Src, Dst) >= OneHalf;
+ Taken = getEdgeProbability(Src, Dst) >= OneHalf;
} else {
// The branch target is not a basic block (most likely a function).
// Since BPI only gives probabilities for targets that are basic blocks,
@@ -3521,7 +3375,7 @@ int HexagonInstrInfo::getDotNewPredJumpO
for (const MachineBasicBlock *SB : B.successors()) {
if (!B.isLayoutSuccessor(SB))
continue;
- Taken = MBPI->getEdgeProbability(Src, SB) < OneHalf;
+ Taken = getEdgeProbability(Src, SB) < OneHalf;
break;
}
} else {
@@ -3534,7 +3388,7 @@ int HexagonInstrInfo::getDotNewPredJumpO
BT = Op.getMBB();
break;
}
- Taken = BT && MBPI->getEdgeProbability(Src, BT) < OneHalf;
+ Taken = BT && getEdgeProbability(Src, BT) < OneHalf;
}
} // if (!Bad)
}
Modified: llvm/trunk/lib/Target/Hexagon/HexagonInstrInfo.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/Hexagon/HexagonInstrInfo.h?rev=304442&r1=304441&r2=304442&view=diff
==============================================================================
--- llvm/trunk/lib/Target/Hexagon/HexagonInstrInfo.h (original)
+++ llvm/trunk/lib/Target/Hexagon/HexagonInstrInfo.h Thu Jun 1 13:02:40 2017
@@ -314,11 +314,6 @@ public:
bool isAccumulator(const MachineInstr &MI) const;
bool isComplex(const MachineInstr &MI) const;
bool isCompoundBranchInstr(const MachineInstr &MI) const;
- bool isCondInst(const MachineInstr &MI) const;
- bool isConditionalALU32 (const MachineInstr &MI) const;
- bool isConditionalLoad(const MachineInstr &MI) const;
- bool isConditionalStore(const MachineInstr &MI) const;
- bool isConditionalTransfer(const MachineInstr &MI) const;
bool isConstExtended(const MachineInstr &MI) const;
bool isDeallocRet(const MachineInstr &MI) const;
bool isDependent(const MachineInstr &ProdMI,
Modified: llvm/trunk/lib/Target/Hexagon/HexagonVLIWPacketizer.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/Hexagon/HexagonVLIWPacketizer.cpp?rev=304442&r1=304441&r2=304442&view=diff
==============================================================================
--- llvm/trunk/lib/Target/Hexagon/HexagonVLIWPacketizer.cpp (original)
+++ llvm/trunk/lib/Target/Hexagon/HexagonVLIWPacketizer.cpp Thu Jun 1 13:02:40 2017
@@ -273,25 +273,17 @@ bool HexagonPacketizerList::isCallDepend
if (DepReg == HRI->getFrameRegister() || DepReg == HRI->getStackRegister())
return true;
- // Check if this is a predicate dependence.
- const TargetRegisterClass* RC = HRI->getMinimalPhysRegClass(DepReg);
- if (RC == &Hexagon::PredRegsRegClass)
- return true;
-
- // Assumes that the first operand of the CALLr is the function address.
- if (HII->isIndirectCall(MI) && (DepType == SDep::Data)) {
- const MachineOperand MO = MI.getOperand(0);
- if (MO.isReg() && MO.isUse() && (MO.getReg() == DepReg))
- return true;
+ // Call-like instructions can be packetized with preceding instructions
+ // that define registers implicitly used or modified by the call. Explicit
+ // uses are still prohibited, as in the case of indirect calls:
+ // r0 = ...
+ // J2_jumpr r0
+ if (DepType == SDep::Data) {
+ for (const MachineOperand MO : MI.operands())
+ if (MO.isReg() && MO.getReg() == DepReg && !MO.isImplicit())
+ return true;
}
- if (HII->isJumpR(MI)) {
- const MachineOperand &MO = HII->isPredicated(MI) ? MI.getOperand(1)
- : MI.getOperand(0);
- assert(MO.isReg() && MO.isUse());
- if (MO.getReg() == DepReg)
- return true;
- }
return false;
}
@@ -333,11 +325,13 @@ bool HexagonPacketizerList::isNewifiable
const TargetRegisterClass *NewRC) {
// Vector stores can be predicated, and can be new-value stores, but
// they cannot be predicated on a .new predicate value.
- if (NewRC == &Hexagon::PredRegsRegClass)
+ if (NewRC == &Hexagon::PredRegsRegClass) {
if (HII->isHVXVec(MI) && MI.mayStore())
return false;
- return HII->isCondInst(MI) || HII->isJumpR(MI) || MI.isReturn() ||
- HII->mayBeNewStore(MI);
+ return HII->isPredicated(MI) && HII->getDotNewPredOp(MI, nullptr) > 0;
+ }
+ // If the class is not PredRegs, it could only apply to new-value stores.
+ return HII->mayBeNewStore(MI);
}
// Promote an instructiont to its .cur form.
@@ -760,11 +754,14 @@ bool HexagonPacketizerList::canPromoteTo
return false;
}
-static bool isImplicitDependency(const MachineInstr &I, unsigned DepReg) {
+static bool isImplicitDependency(const MachineInstr &I, bool CheckDef,
+ unsigned DepReg) {
for (auto &MO : I.operands()) {
- if (MO.isRegMask() && MO.clobbersPhysReg(DepReg))
+ if (CheckDef && MO.isRegMask() && MO.clobbersPhysReg(DepReg))
return true;
- if (MO.isReg() && MO.isDef() && (MO.getReg() == DepReg) && MO.isImplicit())
+ if (!MO.isReg() || MO.getReg() != DepReg || !MO.isImplicit())
+ continue;
+ if (CheckDef == MO.isDef())
return true;
}
return false;
@@ -798,7 +795,8 @@ bool HexagonPacketizerList::canPromoteTo
// If dependency is trough an implicitly defined register, we should not
// newify the use.
- if (isImplicitDependency(PI, DepReg))
+ if (isImplicitDependency(PI, true, DepReg) ||
+ isImplicitDependency(MI, false, DepReg))
return false;
const MCInstrDesc& MCID = PI.getDesc();
@@ -808,8 +806,7 @@ bool HexagonPacketizerList::canPromoteTo
// predicate .new
if (RC == &Hexagon::PredRegsRegClass)
- if (HII->isCondInst(MI) || HII->isJumpR(MI) || MI.isReturn())
- return HII->predCanBeUsedAsDotNew(PI, DepReg);
+ return HII->predCanBeUsedAsDotNew(PI, DepReg);
if (RC != &Hexagon::PredRegsRegClass && !HII->mayBeNewStore(MI))
return false;
Added: llvm/trunk/test/CodeGen/Hexagon/invalid-dotnew-attempt.mir
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/Hexagon/invalid-dotnew-attempt.mir?rev=304442&view=auto
==============================================================================
--- llvm/trunk/test/CodeGen/Hexagon/invalid-dotnew-attempt.mir (added)
+++ llvm/trunk/test/CodeGen/Hexagon/invalid-dotnew-attempt.mir Thu Jun 1 13:02:40 2017
@@ -0,0 +1,17 @@
+# RUN: llc -march=hexagon -start-after if-converter %s -o - | FileCheck %s
+# CHECK: p0 = r0
+# CHECK-NEXT: jumpr r31
+
+# Make sure that the packetizer does not attempt to newify the J2_jumpr
+# only because of the def-use of p0.
+
+---
+name: fred
+tracksRegLiveness: true
+body: |
+ bb.0:
+ liveins: %d0
+ %p0 = C2_tfrrp %r0
+ J2_jumpr %r31, implicit-def %pc, implicit %p0
+...
+
More information about the llvm-commits
mailing list