[llvm] de6e968 - TableGen: Fix logic for default operands

Matt Arsenault via llvm-commits llvm-commits at lists.llvm.org
Wed Feb 19 21:07:23 PST 2020


Author: Matt Arsenault
Date: 2020-02-19T23:41:07-05:00
New Revision: de6e968c0d44d21b16da9b6ec07599e529b8482a

URL: https://github.com/llvm/llvm-project/commit/de6e968c0d44d21b16da9b6ec07599e529b8482a
DIFF: https://github.com/llvm/llvm-project/commit/de6e968c0d44d21b16da9b6ec07599e529b8482a.diff

LOG: TableGen: Fix logic for default operands

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.

Added: 
    

Modified: 
    llvm/test/TableGen/DefaultOpsGlobalISel.td
    llvm/utils/TableGen/CodeGenDAGPatterns.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/test/TableGen/DefaultOpsGlobalISel.td b/llvm/test/TableGen/DefaultOpsGlobalISel.td
index 9ea05e0c3bef..13ee2631ecb0 100644
--- a/llvm/test/TableGen/DefaultOpsGlobalISel.td
+++ b/llvm/test/TableGen/DefaultOpsGlobalISel.td
@@ -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">,
@@ -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)
@@ -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,
@@ -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 
diff erent 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)
+>;

diff  --git a/llvm/utils/TableGen/CodeGenDAGPatterns.cpp b/llvm/utils/TableGen/CodeGenDAGPatterns.cpp
index 0ea90d7dbb65..f9fc046e4b36 100644
--- a/llvm/utils/TableGen/CodeGenDAGPatterns.cpp
+++ b/llvm/utils/TableGen/CodeGenDAGPatterns.cpp
@@ -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.
@@ -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,


        


More information about the llvm-commits mailing list