[PATCH] D32527: Generalize flag carrying SDNodes beyond binary ops. NFC.

[Amara Emerson via Phabricator via llvm-commits]

aemerson added a comment.

In https://reviews.llvm.org/D32527#738046, @spatel wrote:

> In https://reviews.llvm.org/D32527#738020, @aemerson wrote:
>
> > In https://reviews.llvm.org/D32527#738013, @spatel wrote:
> >
> > > Is there an advantage to this vs. just putting the flags in the base SDNode class? We're going to eventually want to use the flags for more than unary and binary nodes (eg, FMA), so I'd prefer to just go directly to that step.
> >
> >
> > I'm not the original author of the code but I think this is due to the extra 2 bytes of storage needed for the flags in each SDNode. With the current solution we only incur this cost if we have flags to store.
>
>
> It's been a long time since https://reviews.llvm.org/D8900, but IIRC, the size argument was actually moot because we have "free" bytes based on the struct alignment (at least on common 64-bit systems). As mentioned there, the fact that the flags were only on binops was a limitation that we wanted to lift even back then, but it just hasn't been done yet. If you can do that in this patch, it would be great. :)


Sure, I'll look at doing that.


Repository:
  rL LLVM

https://reviews.llvm.org/D32527