This greatly simplifies our handling of SDNode::SubclassData.
NFC, hopefully. :)
Paths
| Differential D23036
[SelectionDAG] Use a union of bitfield structs for SDNode::SubclassData. ClosedPublic Authored by jlebar on Aug 1 2016, 2:33 PM.
Details Summary This greatly simplifies our handling of SDNode::SubclassData. NFC, hopefully. :)
Diff Detail Event Timelinejlebar added a parent revision: D23035: [Support] Add LLVM_BITFIELD_WIDTH and LLVM_CHECKED_BITFIELD_ASSIGN macros..Aug 1 2016, 2:33 PM jlebar removed a parent revision: D23035: [Support] Add LLVM_BITFIELD_WIDTH and LLVM_CHECKED_BITFIELD_ASSIGN macros..Aug 2 2016, 2:26 PM
Comment Actions Get rid of getters/setters entirely. FWIW I'm not totally wild about this, because the assertions are verbose. It But I'm happy to leave this how it is too. jlebar added inline comments.
Comment Actions Mostly minor stuff. This is pretty close I think.
jlebar marked 7 inline comments as done. jlebar edited edge metadata. Comment ActionsSecond round of chandlerc's review. Comment Actions Thank you for the review, Chandler.
This revision is now accepted and ready to land.Aug 23 2016, 10:16 AM Closed by commit rL279537: [SelectionDAG] Use a union of bitfield structs for SDNode::SubclassData. (authored by jlebar). · Explain WhyAug 23 2016, 10:26 AM This revision was automatically updated to reflect the committed changes. jlebar marked 2 inline comments as done. ahatanak added inline comments.
Comment Actions Justin, I have to fix this soon since there is an internal bot that is failing the assert, which I suspect is happening because sizeof(SDNodeBits) returns 1. It doesn't look like the correctness of the code depends on sizeof(SDNodeBits) being equal to sizeof(unit16_t), so I'll just remove the assert or replace "==" with "<=". Comment Actions Fixed the static_assert in r279797. Feel free to change the message or remove the assert if you want. I'll be watching the bot to see if it passes the static_assert.
Revision Contents
Diff 68302 llvm/include/llvm/CodeGen/SelectionDAGNodes.h
llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
|
How much are getters and setters at this layer really providing?
If we need these, I'd expect to want them in the actual SDNode public API. For participants in the SDNode type hierarchy, they don't seem to add a lot of value over directly referencing the bitfield.