[RFC] TableGen-erate SDNode descriptions #119709
Conversation
This comment was marked as off-topic.
This comment was marked as off-topic.
s-barannikov
force-pushed
the
tablegen/sdnode-info-emitter
branch
from
2a57488 to
b90b16f
Compare
This comment was marked as off-topic.
This comment was marked as off-topic.
| /// Test if this node has a target-specific | ||
| /// memory-referencing opcode (in the \<target\>ISD namespace and | ||
| /// greater than FIRST_TARGET_MEMORY_OPCODE). | ||
| bool isTargetMemoryOpcode() const { |
There was a problem hiding this comment.
Do we still need isTargetMemoryOpcode(). It doesn't have any callers.
There was a problem hiding this comment.
It has one use in the assert in SelectionDAG::getMemIntrinsicNode.
There was a problem hiding this comment.
Oops. I was looking in my local repo and should have checked for FIRST_TARGET_MEMORY_OPCODE to see that assert
There was a problem hiding this comment.
If nothing else is checking it, does that assert provide any value? Can we just allow any target opcode to be a memory opcode?
There was a problem hiding this comment.
I remembered it has one more "use". Before this patch it is implicitly checked here. isTargetStrictFPOpcode returns true for memory opcodes as well. mayRaiseFPException is then used here to decide whether the resulting MachineInstr needs NoFPExcept flag. IIUC it will not set this flag on MachineInstrs if the matched SDNode(s) were flagged as memory accessing (or having strict-fp semantics).
This behavior was introduced in 6333679. I couldn't conclude if it is safe to return false for memory opcodes so I left it as is.
After this patch the check is explicit, please see SelectionDAGTargetInfo.cpp:mayRaiseFPException.
There was a problem hiding this comment.
I ran CodeGen tests with the memory opcode check disabled and there was one failure in
X86/fp-intrinsics-flags.ll. The following diff resolves it:
There was a discussion in https://reviews.llvm.org/D72466 that can be related to this. Can you confirm that this is a safe change? If so, then quoting @uweigand from https://reviews.llvm.org/D71841 I assume the check for memory opcode can be dropped:
Note that there a bit of a quirk in identifying target nodes that are both memory nodes and strict FP nodes. To simplify the logic, right now all target memory nodes are automatically also considered strict FP nodes -- this could be fixed by adding one more range.
s-barannikov
force-pushed
the
tablegen/sdnode-info-emitter
branch
from
b90b16f to
ad38786
Compare
A future patch adds a couple of new methods to this class, which will need to be overridden by these targets. Part of llvm#119709.
s-barannikov
force-pushed
the
tablegen/sdnode-info-emitter
branch
from
ad38786 to
7ed14ef
Compare
…de (#119969) With this change, targets are no longer required to put memory / strict-fp opcodes after special `ISD::FIRST_TARGET_MEMORY_OPCODE`/`ISD::FIRST_TARGET_STRICTFP_OPCODE` markers. This will also allow autogenerating `isTargetMemoryOpcode`/`isTargetStrictFPOpcode (#119709). Pull Request: #119969
s-barannikov
force-pushed
the
tablegen/sdnode-info-emitter
branch
3 times, most recently
from
7dd3721 to
c315879
Compare
s-barannikov
force-pushed
the
tablegen/sdnode-info-emitter
branch
2 times, most recently
from
5d1058f to
3ba779a
Compare
This patch adds a simplistic backend that gathers all target-specific SelectionDAG nodes and emits descriptions for most of them. This includes generating node enumeration, node names, and information about node "prototype" that can be used to verify that a node is valid. The patch also extends SDNode by adding target-specific flags, which are also included in the generated tables. Part of llvm#119709, [RFC](https://discourse.llvm.org/t/rfc-tablegen-erating-sdnode-descriptions/83627).
This patch adds a simplistic backend that gathers all target-specific SelectionDAG nodes and emits descriptions for most of them. This includes generating node enumeration, node names, and information about node "prototype" that can be used to verify that a node is valid. The patch also extends SDNode by adding target-specific flags, which are also included in the generated tables. Part of llvm#119709, [RFC](https://discourse.llvm.org/t/rfc-tablegen-erating-sdnode-descriptions/83627).
This patch adds a simplistic backend that gathers all target-specific SelectionDAG nodes and emits descriptions for most of them. This includes generating node enumeration, node names, and information about node "prototype" that can be used to verify that a node is valid. The patch also extends SDNode by adding target-specific flags, which are also included in the generated tables. Part of #119709, [RFC](https://discourse.llvm.org/t/rfc-tablegen-erating-sdnode-descriptions/83627). Pull Request: #123002
s-barannikov
force-pushed
the
tablegen/sdnode-info-emitter
branch
from
3ba779a to
4c7893b
Compare
s-barannikov
force-pushed
the
tablegen/sdnode-info-emitter
branch
from
4c7893b to
ae89705
Compare
This patch introduces SelectionDAGGenTargetInfo and SDNodeInfo classes, which provide methods for accessing the generated SDNode descriptions. RFC: https://discourse.llvm.org/t/rfc-tablegen-erating-sdnode-descriptions Draft PR: llvm#119709
This consolidates node definitions into one place and enables automatic node verification. Part of llvm#119709.
This consolidates node definitions into one place and enables automatic node verification. Part of #119709.
s-barannikov
force-pushed
the
tablegen/sdnode-info-emitter
branch
from
76ad0e1 to
be8fae8
Compare
This consolidates node definitions into one place and enables automatic node verification. Part of llvm#119709.
This consolidates node definitions into one place and enables automatic node verification. Part of llvm#119709.
This consolidates node definitions into one place and enables automatic node verification. Part of llvm#119709.
This consolidates node definitions into one place and enables automatic node verification. Part of #119709.
|
I've attempted to continue the AArch64 patch in #140472 (in line with https://discourse.llvm.org/t/rfc-tablegen-erating-sdnode-descriptions/83627/8 🙂) |
s-barannikov
force-pushed
the
tablegen/sdnode-info-emitter
branch
2 times, most recently
from
f5d2b80 to
d69ec4f
Compare
This consolidates node definitions into one place and enables automatic node verification. Part of llvm#119709.
This continues s-barannikov's work TableGen-erating SDNode descriptions. This takes the initial patch from #119709 and moves documentation and the rest of the AArch64ISD nodes to TableGen. Some issues were found by the generated SDNode verification added in this patch. These issues have been described and fixed in the following PRs: - #140706 - #140711 - #140713 - #140715 --------- Co-authored-by: Sergei Barannikov <[email protected]>
s-barannikov
force-pushed
the
tablegen/sdnode-info-emitter
branch
from
d69ec4f to
eec0335
Compare
RFC
Landed changes:
#138381 RISCV
#138450 Sparc
#138869 XCore
#138874 ARC, CSKY, Lanai
#138878 MSP430
#139407 AVR
#139449 M68k
#140472 AArch64