llvm dev - Dec 2020 - Type interference optimization in -gen-dag-isel not working correctly with SDTCisVT and HW modes?

In RISC-V we have these nodes and type constraint.

def SDT_RISCVGREVGORC  : SDTypeProfile<1, 2, [SDTCisVT<0, XLenVT>,
                                              SDTCisSameAs<0, 1>,
                                              SDTCisSameAs<1, 2>]>;
def riscv_grevi    : SDNode<"RISCVISD::GREVI", SDT_RISCVGREVGORC,
[]>;
def riscv_greviw   : SDNode<"RISCVISD::GREVIW", SDT_RISCVGREVGORC,
[]>;
def riscv_gorci    : SDNode<"RISCVISD::GORCI", SDT_RISCVGREVGORC,
[]>;
def riscv_gorciw   : SDNode<"RISCVISD::GORCIW", SDT_RISCVGREVGORC,
[]>;

Where XLenVT is dependent on HW mode. My expectation is that a
pattern using one of these nodes would need no type checks in the
isel table since the type is fixed for a particular HW mode. Even without
the fixed type, I would expect the two SDTCisSameAs to get it down to 1
type check. But this is what I see in the DAG isel table.

/* 31138*/ /*SwitchOpcode*/ 49, TARGET_VAL(RISCVISD::GREVI),// ->31190
/* 31141*/  OPC_RecordChild0, // #0 = $rs1
/* 31142*/  OPC_Scope, 22, /*->31166*/ // 2 children in Scope
/* 31144*/   OPC_CheckChild0Type, MVT::i64,
/* 31146*/   OPC_RecordChild1, // #1 = $shamt
/* 31147*/   OPC_MoveChild1,
/* 31148*/   OPC_CheckOpcode, TARGET_VAL(ISD::TargetConstant),
/* 31151*/   OPC_CheckType, MVT::i64,
/* 31153*/   OPC_MoveParent,
/* 31154*/   OPC_CheckType, MVT::i64,
/* 31156*/   OPC_CheckPatternPredicate, 49, // (Subtarget->hasStdExtZbp())
&& (MF->getSubtarget().checkFeatures("+64bit"))
/* 31158*/   OPC_MorphNodeTo1, TARGET_VAL(RISCV::GREVI), 0,
                 MVT::i64, 2/*#Ops*/, 0, 1,
             // Src: (riscv_grevi:{ *:[i64] } GPR:{ *:[i64] }:$rs1, (timm:{
*:[i64] }):$shamt) - Complexity = 6
             // Dst: (GREVI:{ *:[i64] } GPR:{ *:[i64] }:$rs1, (timm:{
*:[i64] }):$shamt)
/* 31166*/  /*Scope*/ 22, /*->31189*/
/* 31167*/   OPC_CheckChild0Type, MVT::i32,
/* 31169*/   OPC_RecordChild1, // #1 = $shamt
/* 31170*/   OPC_MoveChild1,
/* 31171*/   OPC_CheckOpcode, TARGET_VAL(ISD::TargetConstant),
/* 31174*/   OPC_CheckType, MVT::i32,
/* 31176*/   OPC_MoveParent,
/* 31177*/   OPC_CheckType, MVT::i32,
/* 31179*/   OPC_CheckPatternPredicate, 50, // (Subtarget->hasStdExtZbp())
/* 31181*/   OPC_MorphNodeTo1, TARGET_VAL(RISCV::GREVI), 0,
                 MVT::i32, 2/*#Ops*/, 0, 1,
             // Src: (riscv_grevi:{ *:[i32] } GPR:{ *:[i32] }:$rs1, (timm:{
*:[i32] }):$shamt) - Complexity = 6
             // Dst: (GREVI:{ *:[i32] } GPR:{ *:[i32] }:$rs1, (timm:{
*:[i32] }):$shamt)

There are 3 type checks for i64 in one HW mode and 3 checks for i32 in the
other HW mode. One for each operand and one for the result.

I'm probably going to change this to use SDTIntBinOp and get rid of the
custom type constraints. That works correctly and generates one type check.
It's probably not worth being more specific than that for this case even if
it was working correctly. I wanted to bring up the issue since it might be
affecting other patterns.

Possibly related, while I was briefly trying to investigate this I noticed
that ForceMode in the tablegen backend's TypeInfer class is only ever
assigned and never read from. And the DefaultPred std::vector
in CodeGenDAGPatterns::ExpandHwModeBasedTypes() is also never read from.

~Craig
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20201205/65336ea6/attachment.html>

Would you like me to investigate the issues mentioned in your final paragraph?

At 12/5/2020 09:47 PM, Craig Topper wrote:
>There are 3 type checks for i64 in one HW mode and 3 checks for i32 in the
other HW mode. One for each operand and one for the result.
>
>I'm probably going to change this to use SDTIntBinOp and get rid of the
custom type constraints. That works correctly and generates one type check.
It's probably not worth being more specific than that for this case even if
it was working correctly. I wanted to bring up the issue since it might be
affecting other patterns.
>
>Possibly related, while I was briefly trying to investigate this I noticed
that ForceMode in the tablegen backend's TypeInfer class is only ever
assigned and never read from. And the DefaultPred std::vector inÂ
CodeGenDAGPatterns::ExpandHwModeBasedTypes() is also never read from.
>
>~Craig