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

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

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?



================
Comment at: clang/include/clang/Basic/LangOptions.h:200
     /// Rounding to nearest, corresponds to "round.tonearest".
-    FPR_ToNearest,
+    FPR_ToNearest = 0,
     /// Rounding toward -Inf, corresponds to "round.downward".
----------------
this is implied and unneccesary


================
Comment at: clang/include/clang/Basic/LangOptions.h:361
                 fenv_access(LangOptions::FEA_Off),
-                rounding(LangOptions::FPR_ToNearest),
-                exceptions(LangOptions::FPE_Ignore)
+                packed(0)
         {}
----------------
Making use of the encoding of `packed` in lots of places is fragile and hard to maintain.

I'd suggest the private primitives:
`setRoundingAndExceptionMode(FPRoundingModeKind, FPExceptionModeKind)`
`getRoundingAndExceptionMode() -> pair<FPRoundingModeKind, FPExceptionModeKind>`
and implementing everything in terms of them.



================
Comment at: clang/include/clang/Basic/LangOptions.h:445
+  // A packed field for encoding rounding and exceptions.
+  // packed = MaxExceptionValue * rounding + exceptions.
+  constexpr static unsigned MaxExceptionValue = 3;
----------------
FIXME: unpack this once saving one bit isn't critical here.


================
Comment at: clang/include/clang/Basic/LangOptions.h:447
+  constexpr static unsigned MaxExceptionValue = 3;
+  unsigned packed: 4;
 };
----------------
rounding_and_exceptions?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75443