Skip to content

Commit

Permalink
TableGen: Fix logic for default operands
Browse files Browse the repository at this point in the history
This was checking for default operands in the current DAG instruction,
rather than the correct result operand list. I'm not entirly sure how
this managed to work before, but was failing for me when multiple
default operands were overridden.
  • Loading branch information
arsenm committed Feb 20, 2020
1 parent e1eed6c commit de6e968
Show file tree
Hide file tree
Showing 2 changed files with 52 additions and 5 deletions.
43 changes: 43 additions & 0 deletions llvm/test/TableGen/DefaultOpsGlobalISel.td
Expand Up @@ -7,6 +7,7 @@ include "GlobalISelEmitterCommon.td"
def SelectClamp : ComplexPattern<untyped, 2, "SelectClamp">;
def SelectOMod : ComplexPattern<untyped, 2, "SelectOMod">;
def SelectClampOMod : ComplexPattern<untyped, 3, "SelectClampOMod">;
def SelectSrcMods : ComplexPattern<untyped, 2, "SelectSrcMods">;

def gi_SelectClamp :
GIComplexOperandMatcher<s32, "selectClamp">,
Expand All @@ -20,11 +21,29 @@ def gi_SelectClampOMod :
GIComplexOperandMatcher<s32, "selectClampOMod">,
GIComplexPatternEquiv<SelectClampOMod>;

def gi_SelectSrcMods :
GIComplexOperandMatcher<s32, "selectSrcMods">,
GIComplexPatternEquiv<SelectSrcMods>;


def src_mods : Operand <i32>;
def omod : OperandWithDefaultOps <i32, (ops (i32 0))>;
def clamp : OperandWithDefaultOps <i1, (ops (i1 0))>;


// CHECK: GIM_CheckOpcode, /*MI*/0, TargetOpcode::G_FMAXNUM,
// CHECK: GIM_CheckComplexPattern, /*MI*/0, /*Op*/1, /*Renderer*/0, GICP_gi_SelectSrcMods,
// CHECK: GIM_CheckComplexPattern, /*MI*/0, /*Op*/2, /*Renderer*/1, GICP_gi_SelectSrcMods,
// CHECK: GIR_BuildMI, /*InsnID*/0, /*Opcode*/MyTarget::FMAX,
// CHECK-NEXT: GIR_Copy, /*NewInsnID*/0, /*OldInsnID*/0, /*OpIdx*/0, // dst
// CHECK-NEXT: GIR_ComplexSubOperandRenderer, /*InsnID*/0, /*RendererID*/0, /*SubOperand*/1, // mods0
// CHECK-NEXT: GIR_ComplexSubOperandRenderer, /*InsnID*/0, /*RendererID*/0, /*SubOperand*/0, // src0
// CHECK-NEXT: GIR_ComplexSubOperandRenderer, /*InsnID*/0, /*RendererID*/1, /*SubOperand*/1, // mods1
// CHECK-NEXT: GIR_ComplexSubOperandRenderer, /*InsnID*/0, /*RendererID*/1, /*SubOperand*/0, // src1
// CHECK-NEXT: GIR_AddImm, /*InsnID*/0, /*Imm*/0,
// CHECK-NEXT: GIR_EraseFromParent, /*InsnID*/0,


// CHECK: GIM_CheckOpcode, /*MI*/0, TargetOpcode::G_FFLOOR,
// CHECK: GIM_CheckComplexPattern, /*MI*/0, /*Op*/1, /*Renderer*/0, GICP_gi_SelectClampOMod,
// CHECK: // (ffloor:{ *:[f32] } (SelectClampOMod:{ *:[f32] } f32:{ *:[f32] }:$src0, omod:{ *:[i32] }:$omod, i1:{ *:[i1] }:$clamp)) => (FLOMP:{ *:[f32] } f32:{ *:[f32] }:$src0, i1:{ *:[i1] }:$clamp, omod:{ *:[i32] }:$omod)
Expand All @@ -35,6 +54,17 @@ def clamp : OperandWithDefaultOps <i1, (ops (i1 0))>;
// CHECK-NEXT: GIR_ComplexSubOperandRenderer, /*InsnID*/0, /*RendererID*/0, /*SubOperand*/1, // omod


// CHECK: GIM_CheckOpcode, /*MI*/0, TargetOpcode::G_FCANONICALIZE,
// CHECK: GIR_BuildMI, /*InsnID*/0, /*Opcode*/MyTarget::FMAX,
// CHECK-NEXT: GIR_Copy, /*NewInsnID*/0, /*OldInsnID*/0, /*OpIdx*/0, // dst
// CHECK-NEXT: GIR_ComplexSubOperandRenderer, /*InsnID*/0, /*RendererID*/0, /*SubOperand*/1, // mods
// CHECK-NEXT: GIR_ComplexSubOperandRenderer, /*InsnID*/0, /*RendererID*/0, /*SubOperand*/0, // src
// CHECK-NEXT: GIR_ComplexSubOperandRenderer, /*InsnID*/0, /*RendererID*/0, /*SubOperand*/1, // mods
// CHECK-NEXT: GIR_ComplexSubOperandRenderer, /*InsnID*/0, /*RendererID*/0, /*SubOperand*/0, // src
// CHECK-NEXT: GIR_AddImm, /*InsnID*/0, /*Imm*/0,
// CHECK-NEXT: GIR_EraseFromParent, /*InsnID*/0,


// CHECK: GIM_CheckOpcode, /*MI*/0, TargetOpcode::G_FCOS,
// CHECK: // (fcos:{ *:[f32] } (SelectOMod:{ *:[f32] } f32:{ *:[f32] }:$src0, i32:{ *:[i32] }:$omod)) => (FLAMP:{ *:[f32] } FPR32:{ *:[f32] }:$src0, omod:{ *:[i32] }:$omod)
// CHECK-NEXT: GIR_BuildMI, /*InsnID*/0, /*Opcode*/MyTarget::FLAMP,
Expand Down Expand Up @@ -142,3 +172,16 @@ def : Pat <
(fexp2 (SelectClamp f32:$src0, i1:$clamp)),
(FEEPLE FPR32:$src0, (FFOO FPR32:$src0), clamp:$clamp)
>;

// Same instruction is used in two different pattern contexts, one
// uses the default and one does not.
def FMAX : I<(outs FPR32:$dst),
(ins src_mods:$mods0, FPR32:$src0, src_mods:$mods1, FPR32:$src1, clamp:$clamp),
[(set FPR32:$dst, (f32 (fmaxnum (SelectSrcMods f32:$src0, src_mods:$mods0),
(SelectSrcMods f32:$src1, src_mods:$mods1))))]
>;

def : Pat<
(fcanonicalize (f32 (SelectSrcMods f32:$src, i32:$mods))),
(FMAX $mods, $src, $mods, $src, 0)
>;
14 changes: 9 additions & 5 deletions llvm/utils/TableGen/CodeGenDAGPatterns.cpp
Expand Up @@ -2520,6 +2520,9 @@ bool TreePatternNode::ApplyTypeConstraints(TreePattern &TP, bool NotRegisters) {
}
}

unsigned NumResults = Inst.getNumResults();
unsigned NumFixedOperands = InstInfo.Operands.size();

// If one or more operands with a default value appear at the end of the
// formal operand list for an instruction, we allow them to be overridden
// by optional operands provided in the pattern.
Expand All @@ -2528,14 +2531,15 @@ bool TreePatternNode::ApplyTypeConstraints(TreePattern &TP, bool NotRegisters) {
// operand A with a default, then we don't allow A to be overridden,
// because there would be no way to specify whether the next operand in
// the pattern was intended to override A or skip it.
unsigned NonOverridableOperands = Inst.getNumOperands();
while (NonOverridableOperands > 0 &&
CDP.operandHasDefault(Inst.getOperand(NonOverridableOperands-1)))
unsigned NonOverridableOperands = NumFixedOperands;
while (NonOverridableOperands > NumResults &&
CDP.operandHasDefault(InstInfo.Operands[NonOverridableOperands-1].Rec))
--NonOverridableOperands;

unsigned ChildNo = 0;
for (unsigned i = 0, e = Inst.getNumOperands(); i != e; ++i) {
Record *OperandNode = Inst.getOperand(i);
assert(NumResults <= NumFixedOperands);
for (unsigned i = NumResults, e = NumFixedOperands; i != e; ++i) {
Record *OperandNode = InstInfo.Operands[i].Rec;

// If the operand has a default value, do we use it? We must use the
// default if we've run out of children of the pattern DAG to consume,
Expand Down

0 comments on commit de6e968

Please sign in to comment.