[PATCH] D23036: [SelectionDAG] Use a union of bitfield structs for SDNode::SubclassData.

[Justin Lebar via llvm-commits]

Sorry for taking so long to come back to this; I was sick after I got
back from my trip.

Now that I look at it, I don't think the new code is right.

   unsigned getRawSubclassData() const {
     uint16_t Data;
     memcpy(&Data, &SDNodeBits, sizeof(SDNodeBits));
     static_assert(sizeof(SDNodeBits) <= sizeof(uint16_t),
       "SDNodeBits field too large?");
     return Data;
   }

If sizeof(SDNodeBits) == 1, then we have a few problems:

* Our code is endianness-unsafe.  Which doesn't matter because we're
just using this as input into a hashtable key, but still.
* Half of Data is uninitialized.
* We're not copying all of the bits from the anonymous union, which is
the intent of the code.

The obvious thing would be to add a uint16_t member to the anon union,
but I am pretty sure Chandler explained to me this would be UB.  We
are already walking very close to the line here.

I'll send a patch to do the not-obvious thing and make sure a language
lawyer stamps it.

-Justin

On Thu, Aug 25, 2016 at 5:57 PM, Akira Hatanaka <ahatanak at gmail.com> wrote:
> ahatanak added a comment.
>
> 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.
>
>
> Repository:
>   rL LLVM
>
> https://reviews.llvm.org/D23036
>
>
>