[llvm] r303240 - [GlobalISel][TableGen] Fix handling of default operands
Diana Picus via llvm-commits
llvm-commits at lists.llvm.org
Wed May 17 01:57:30 PDT 2017
Author: rovka
Date: Wed May 17 03:57:28 2017
New Revision: 303240
URL: http://llvm.org/viewvc/llvm-project?rev=303240&view=rev
Log:
[GlobalISel][TableGen] Fix handling of default operands
When looping through a destination pattern's operands to decide how many
default operands we need to introduce, we used to count the "expanded"
number of operands. So if one default operand would be rendered as 2
values, we'd count it as 2 operands, when in fact it needs to count as
only 1 operand regardless of how many values it expands to.
This turns out to be a problem only in some very specific cases, e.g.
when we have one operand with multiple default values followed by more
operands with default values (see the new test). In such a situation
we'd stop looping before looking at all the operands, and then error out
assuming that we don't have enough default operands to make up the
shortfall.
At the moment this only affects ARM.
The patch removes the loop counting default operands entirely and
assumes that we'll have to introduce values for any default operand that
we find (i.e. we're assuming it cannot be given as a child at all). It
also extracts the code for adding renderers for default operands into a
helper method.
Differential Revision: https://reviews.llvm.org/D33031
Modified:
llvm/trunk/test/TableGen/GlobalISelEmitter.td
llvm/trunk/utils/TableGen/GlobalISelEmitter.cpp
Modified: llvm/trunk/test/TableGen/GlobalISelEmitter.td
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/TableGen/GlobalISelEmitter.td?rev=303240&r1=303239&r2=303240&view=diff
==============================================================================
--- llvm/trunk/test/TableGen/GlobalISelEmitter.td (original)
+++ llvm/trunk/test/TableGen/GlobalISelEmitter.td Wed May 17 03:57:28 2017
@@ -387,6 +387,42 @@ def XOR : I<(outs GPR32:$dst), (ins Z:$s
def XORlike : I<(outs GPR32:$dst), (ins m1Z:$src2, GPR32:$src1),
[(set GPR32:$dst, (xor GPR32:$src1, -4))]>;
+//===- Test a simple pattern with multiple operands with defaults. --------===//
+//
+
+// CHECK-LABEL: if ([&]() {
+// CHECK-NEXT: MachineInstr &MI0 = I;
+// CHECK-NEXT: if (MI0.getNumOperands() < 3)
+// CHECK-NEXT: return false;
+// CHECK-NEXT: if ((MI0.getOpcode() == TargetOpcode::G_XOR) &&
+// CHECK-NEXT: ((/* dst */ (MRI.getType(MI0.getOperand(0).getReg()) == (LLT::scalar(32))) &&
+// CHECK-NEXT: ((&RBI.getRegBankFromRegClass(MyTarget::GPR32RegClass) == RBI.getRegBank(MI0.getOperand(0).getReg(), MRI, TRI))))) &&
+// CHECK-NEXT: ((/* src1 */ (MRI.getType(MI0.getOperand(1).getReg()) == (LLT::scalar(32))) &&
+// CHECK-NEXT: ((&RBI.getRegBankFromRegClass(MyTarget::GPR32RegClass) == RBI.getRegBank(MI0.getOperand(1).getReg(), MRI, TRI))))) &&
+// CHECK-NEXT: ((/* Operand 2 */ (MRI.getType(MI0.getOperand(2).getReg()) == (LLT::scalar(32))) &&
+// CHECK-NEXT: (isOperandImmEqual(MI0.getOperand(2), -5, MRI))))) {
+// CHECK-NEXT: // (xor:i32 GPR32:i32:$src1, -5:i32) => (XORManyDefaults:i32 GPR32:i32:$src1)
+// CHECK-NEXT: MachineInstrBuilder MIB = BuildMI(*I.getParent(), I, I.getDebugLoc(), TII.get(MyTarget::XORManyDefaults));
+// CHECK-NEXT: MIB.add(MI0.getOperand(0)/*dst*/);
+// CHECK-NEXT: MIB.addImm(-1);
+// CHECK-NEXT: MIB.addReg(MyTarget::R0);
+// CHECK-NEXT: MIB.addReg(MyTarget::R0);
+// CHECK-NEXT: MIB.add(MI0.getOperand(1)/*src1*/);
+// CHECK-NEXT: for (const auto *FromMI : {&MI0, })
+// CHECK-NEXT: for (const auto &MMO : FromMI->memoperands())
+// CHECK-NEXT: MIB.addMemOperand(MMO);
+// CHECK-NEXT: I.eraseFromParent();
+// CHECK-NEXT: MachineInstr &NewI = *MIB;
+// CHECK-NEXT: constrainSelectedInstRegOperands(NewI, TII, TRI, RBI);
+// CHECK-NEXT: return true;
+// CHECK-NEXT: }
+// CHECK-NEXT: return false;
+// CHECK-NEXT: }()) { return true; }
+
+// The -5 is just to distinguish it from the other cases.
+def XORManyDefaults : I<(outs GPR32:$dst), (ins m1Z:$src3, Z:$src2, GPR32:$src1),
+ [(set GPR32:$dst, (xor GPR32:$src1, -5))]>;
+
//===- Test a simple pattern with constant immediate operands. ------------===//
//
// This must precede the 3-register variants because constant immediates have
Modified: llvm/trunk/utils/TableGen/GlobalISelEmitter.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/utils/TableGen/GlobalISelEmitter.cpp?rev=303240&r1=303239&r2=303240&view=diff
==============================================================================
--- llvm/trunk/utils/TableGen/GlobalISelEmitter.cpp (original)
+++ llvm/trunk/utils/TableGen/GlobalISelEmitter.cpp Wed May 17 03:57:28 2017
@@ -1242,6 +1242,8 @@ private:
Error importExplicitUseRenderer(BuildMIAction &DstMIBuilder,
TreePatternNode *DstChild,
const InstructionMatcher &InsnMatcher) const;
+ Error importDefaultOperandRenderers(BuildMIAction &DstMIBuilder,
+ DagInit *DefaultOps) const;
Error
importImplicitDefRenderers(BuildMIAction &DstMIBuilder,
const std::vector<Record *> &ImplicitDefs) const;
@@ -1509,59 +1511,23 @@ Expected<BuildMIAction &> GlobalISelEmit
DstMIBuilder.addRenderer<CopyRenderer>(InsnMatcher, DstIOperand.Name);
}
- // Figure out which operands need defaults inserted. Operands that subclass
- // OperandWithDefaultOps are considered from left to right until we have
- // enough operands to render the instruction.
- SmallSet<unsigned, 2> DefaultOperands;
- unsigned DstINumUses = DstI.Operands.size() - DstI.Operands.NumDefs;
- unsigned NumDefaultOperands = 0;
- for (unsigned I = 0; I < DstINumUses &&
- DstINumUses > Dst->getNumChildren() + NumDefaultOperands;
- ++I) {
- const auto &DstIOperand = DstI.Operands[DstI.Operands.NumDefs + I];
- if (DstIOperand.Rec->isSubClassOf("OperandWithDefaultOps")) {
- DefaultOperands.insert(I);
- NumDefaultOperands +=
- DstIOperand.Rec->getValueAsDag("DefaultOps")->getNumArgs();
- }
- }
- if (DstINumUses > Dst->getNumChildren() + DefaultOperands.size())
- return failedImport("Insufficient operands supplied and default ops "
- "couldn't make up the shortfall");
- if (DstINumUses < Dst->getNumChildren() + DefaultOperands.size())
- return failedImport("Too many operands supplied");
-
// Render the explicit uses.
unsigned Child = 0;
+ unsigned DstINumUses = DstI.Operands.size() - DstI.Operands.NumDefs;
+ unsigned NumDefaultOps = 0;
for (unsigned I = 0; I != DstINumUses; ++I) {
- // If we need to insert default ops here, then do so.
- if (DefaultOperands.count(I)) {
- const auto &DstIOperand = DstI.Operands[DstI.Operands.NumDefs + I];
+ const auto &DstIOperand = DstI.Operands[DstI.Operands.NumDefs + I];
+ // If the operand has default values, introduce them now.
+ // FIXME: Until we have a decent test case that dictates we should do
+ // otherwise, we're going to assume that operands with default values cannot
+ // be specified in the patterns. Therefore, adding them will not cause us to
+ // end up with too many rendered operands.
+ if (DstIOperand.Rec->isSubClassOf("OperandWithDefaultOps")) {
DagInit *DefaultOps = DstIOperand.Rec->getValueAsDag("DefaultOps");
- for (const auto *DefaultOp : DefaultOps->args()) {
- // Look through ValueType operators.
- if (const DagInit *DefaultDagOp = dyn_cast<DagInit>(DefaultOp)) {
- if (const DefInit *DefaultDagOperator =
- dyn_cast<DefInit>(DefaultDagOp->getOperator())) {
- if (DefaultDagOperator->getDef()->isSubClassOf("ValueType"))
- DefaultOp = DefaultDagOp->getArg(0);
- }
- }
-
- if (const DefInit *DefaultDefOp = dyn_cast<DefInit>(DefaultOp)) {
- DstMIBuilder.addRenderer<AddRegisterRenderer>(DefaultDefOp->getDef());
- continue;
- }
-
- if (const IntInit *DefaultIntOp = dyn_cast<IntInit>(DefaultOp)) {
- DstMIBuilder.addRenderer<ImmRenderer>(DefaultIntOp->getValue());
- continue;
- }
-
- return failedImport("Could not add default op");
- }
-
+ if (auto Error = importDefaultOperandRenderers(DstMIBuilder, DefaultOps))
+ return std::move(Error);
+ ++NumDefaultOps;
continue;
}
@@ -1571,9 +1537,44 @@ Expected<BuildMIAction &> GlobalISelEmit
++Child;
}
+ if (NumDefaultOps + Dst->getNumChildren() != DstINumUses)
+ return failedImport("Expected " + std::to_string(DstINumUses) +
+ " used operands but found " +
+ std::to_string(Dst->getNumChildren()) +
+ " explicit ones and " + std::to_string(NumDefaultOps) +
+ " default ones");
+
return DstMIBuilder;
}
+Error GlobalISelEmitter::importDefaultOperandRenderers(
+ BuildMIAction &DstMIBuilder, DagInit *DefaultOps) const {
+ for (const auto *DefaultOp : DefaultOps->args()) {
+ // Look through ValueType operators.
+ if (const DagInit *DefaultDagOp = dyn_cast<DagInit>(DefaultOp)) {
+ if (const DefInit *DefaultDagOperator =
+ dyn_cast<DefInit>(DefaultDagOp->getOperator())) {
+ if (DefaultDagOperator->getDef()->isSubClassOf("ValueType"))
+ DefaultOp = DefaultDagOp->getArg(0);
+ }
+ }
+
+ if (const DefInit *DefaultDefOp = dyn_cast<DefInit>(DefaultOp)) {
+ DstMIBuilder.addRenderer<AddRegisterRenderer>(DefaultDefOp->getDef());
+ continue;
+ }
+
+ if (const IntInit *DefaultIntOp = dyn_cast<IntInit>(DefaultOp)) {
+ DstMIBuilder.addRenderer<ImmRenderer>(DefaultIntOp->getValue());
+ continue;
+ }
+
+ return failedImport("Could not add default op");
+ }
+
+ return Error::success();
+}
+
Error GlobalISelEmitter::importImplicitDefRenderers(
BuildMIAction &DstMIBuilder,
const std::vector<Record *> &ImplicitDefs) const {
More information about the llvm-commits
mailing list