[BPF] TableGen-erate SDNode descriptions

[s-barannikov]

This allows SDNodes to be validated against their expected type profiles
and reduces the number of changes required to add a new node.

Fix BR_CC/MEMCPY descriptions to match C++ code that creates the nodes
(an error detected by the enabled verification functionality).

Also remove redundant SDNPOutGlue on BPFISD::MEMCPY.

Part of #119709.

[github-actions]

✅ With the latest revision this PR passed the C/C++ code formatter.

[yonghong-song]

I tested by building bpf selftest with this change and everything is okay. I compared to XCore change (#138869) and they are very similar to each other.

You mentioned below:

Fix BR_CC/MEMCPY descriptions to match C++ code that creates the nodes
(an error detected by the enabled verification functionality).

Do you know why we do not have problems before for BR_CC/MRMCPY?

I checked a few targets, e.g.

def LanaiBrCC        : SDNode<"LanaiISD::BR_CC", SDT_LanaiBrCC,
                              [SDNPHasChain, SDNPInGlue]>;
def loongarch_brcc : SDNode<"LoongArchISD::BR_CC", SDT_LoongArchBrCC,
                            [SDNPHasChain]>;

and

def ARMmemcopy : SDNode<"ARMISD::MEMCPY", SDT_ARMMEMCPY,
                        [SDNPHasChain, SDNPInGlue, SDNPOutGlue,
                         SDNPMayStore, SDNPMayLoad]>;
def wasm_memcpy : SDNode<"WebAssemblyISD::MEMCPY", wasm_memcpy_t,
                         [SDNPHasChain, SDNPMayLoad, SDNPMayStore]>;

Looks like WASM is correct and the other three need update...

[eddyz87]

Could you please comment a bit on removing SDNPOutGlue, SDNPInGlue and SDNPInGlue below?

[s-barannikov]

Glue operands/results are used when there is a copy to/from a physical register that needs be scheduled right before / next to the node. This isn't the case for BR_CC, BPFTargetLowering::LowerBR_CC() creates it without glue operands/results:

  return DAG.getNode(BPFISD::BR_CC, DL, Op.getValueType(), Chain, LHS, RHS,
                     DAG.getConstant(CC, DL, LHS.getValueType()), Dest);

Removing these properties here is just syncing with C++ code to make SDNodeInfo::verifyNode() happy.

[yonghong-song]

Indeed, for BPFISD::BR_CC, we do not have in/out glue so it makes sense to remove it.

[s-barannikov]

Do you know why we do not have problems before for BR_CC/MRMCPY?

SelectionDAG is very permissive about such inconsistencies. Fixing it to be more robust is my end goal.

Looks like WASM is correct and the other three need update...

LanaiISD::BR_CC uses SDNPInGlue because it "glues" a comparison to the branch. In BPF comparison and branch a fused into one BPFISD::BR_CC node (same as LoongArch).

ARMISD::MEMCPY is indeed incorrect in that it shouldn't have SDNPInGlue. I'm not sure if it needs an out glue, but at least its presence is consistent with C++ code.

Whether or not to use glue operands/results is really target-dependent.

[yonghong-song]

Do you know why we do not have problems before for BR_CC/MRMCPY?

SelectionDAG is very permissive about such inconsistencies. Fixing it to be more robust is my end goal.

Looks like WASM is correct and the other three need update...

LanaiISD::BR_CC uses SDNPInGlue because it "glues" a comparison to the branch. In BPF comparison and branch a fused into one BPFISD::BR_CC node (same as LoongArch).

ARMISD::MEMCPY is indeed incorrect in that it shouldn't have SDNPInGlue. I'm not sure if it needs an out glue, but at least its presence is consistent with C++ code.

Whether or not to use glue operands/results is really target-dependent.

But for BPFmemcpy, currently we have

def BPFmemcpy       : SDNode<"BPFISD::MEMCPY", SDT_BPFMEMCPY,
                             [SDNPHasChain, SDNPInGlue, SDNPOutGlue,
                              SDNPMayStore, SDNPMayLoad]>;

It is used in custom inserter:

let usesCustomInserter = 1, isCodeGenOnly = 1 in {
    def MEMCPY : Pseudo<
      (outs),
      (ins GPR:$dst, GPR:$src, i64imm:$len, i64imm:$align, variable_ops),
      "#memcpy dst: $dst, src: $src, len: $len, align: $align",
      [(BPFmemcpy GPR:$dst, GPR:$src, imm:$len, imm:$align)]>;
} 

This patch only removes SNDPInGlue, I think we should remove SNDPOutGlue as well. WDYT?

[s-barannikov]

This patch only removes SNDPInGlue, I think we should remove SNDPOutGlue as well. WDYT?

Done