[PATCH] D75443: [AST] Unpack FPFeatures bits to BinaryOperator, NFC.

Wed Mar 11 07:24:43 PDT 2020

hokein added a comment.

In D75443#1916829 <https://reviews.llvm.org/D75443#1916829>, @sammccall wrote:

> In D75443#1916761 <https://reviews.llvm.org/D75443#1916761>, @mibintc wrote:
>
> > I'm working on a patch that will move FPOptions to Trailing storage, part of https://reviews.llvm.org/D72841 ; hope to have another revision uploaded today or tomorrow.  I got feedback that moving the field to BinaryOperator isn't adequate.
>
>
> Great! Moving FPOptions to trailing storage (which I guess allows it to only be present for floating point ops?) is much better than making it a member, and frees up a lot of bits.
>
> To unblock adding the extra "error" bit that we need (which really does apply to all Exprs), we could do any of:
>
> 1. wait for D72841 <https://reviews.llvm.org/D72841> to land, including the trailingstorage
> 2. move the trailing-storage parts out of that patch, and do it first as a smaller separate change
> 3. find some other way to save the bit for now (e.g. by encoding rounding + exceptions together), and undo that once saving 1 bit doesn't matter
>
>   Any objections if we do 3 and take care of rolling it back later?

I think waiting for 1 would probably take a long time since that is a huge patch. I'd prefer 3 to unblock the "error" bit stuff, but I'm also happy with 2. what's your thought @mibintc?

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D75443/new/

https://reviews.llvm.org/D75443