Move FPFeatures from BinaryOperator bitfields to Trailing storage

[mibintc]

Summary

This patch moves the FPFeatures field from BinaryOperator bitfields to Trailing storage, since there is a current desire to widen the FPOptions type. It's a preliminary patch, looking for feedback from @rjmccall and others. On the dev email list, there was an alternate proposal to maintain this information in state. I had already partly implemented this change so I just continued along this way.

There's still a problem in the patch, there's a bug in the AST statement visitor, something wrong with the template metaprogramming? i still haven't found that problem, but i wanted to put this up for any feedback before i find and fix that problem, If you see what i did wrong please let me know. The problem has to do with combining the CompoundAssignmentOperator and BinaryOperator. I needed to do that because BinaryOperator needs to be finalized before Trailing storage can be used.

FPFeatures are also needed on CXXOperatorCall. I put that field into CallExpr. Do you want that to go into Trailing storage as well? I didn't do that. It would also require CallExpr and CXXOperatorCall to be collapsed together.

[mibintc]

There's a bug in this patch that i haven't solved yet. I think it's just 1 bug. The bug is that the AST statement visitor is going to the "fallback" instead of the right destination when walking CompoundAssignmentOperator. This patch collapses CompoundAssignmentOperator class into BinaryOperator class. In a spot check, i think all the failing tests are because the Visitor walk isn't working correctly for CompoundAssign (for example += ). These are the test fails reported by check-clang in my sandbox,
Failing Tests (15):

Clang :: Analysis/loopexit-cfg-output.cpp
Clang :: CXX/drs/dr2xx.cpp
Clang :: CXX/expr/expr.const/p2-0x.cpp
Clang :: CXX/expr/expr.prim/expr.prim.lambda/expr.prim.lambda.capture/p17.cpp
Clang :: CodeGen/ffp-contract-fast-option.cpp
Clang :: CodeGenCXX/const-init-cxx1y.cpp
Clang :: CodeGenCXX/const-init-cxx2a.cpp
Clang :: CodeGenCXX/non-const-init-cxx2a.cpp
Clang :: Sema/warn-unreachable.c
Clang :: Sema/warn-unsequenced.c
Clang :: SemaCXX/constant-expression-cxx1y.cpp
Clang :: SemaCXX/constant-expression-cxx2a.cpp
Clang :: SemaCXX/decomposed-condition.cpp
Clang :: SemaCXX/integer-overflow.cpp
Clang :: SemaCXX/warn-unsequenced.cpp

[mibintc]

This revision has been rebased, the LIT test failures seen in the previous revision have been corrected: check-all passes. Still owe you some polishing. I'm going to add a couple inline comments.

[mibintc]

In D76384#1929774, @mibintc wrote:

There's a bug in this patch that i haven't solved yet. I think it's just 1 bug. The bug is that the AST statement visitor is going to the "fallback" instead of the right destination when walking CompoundAssignmentOperator. This patch collapses CompoundAssignmentOperator class into BinaryOperator class. In a spot check, i think all the failing tests are because the Visitor walk isn't working correctly for CompoundAssign (for example += ). These are the test fails reported by check-clang in my sandbox,
Failing Tests (15):

Clang :: Analysis/loopexit-cfg-output.cpp
Clang :: CXX/drs/dr2xx.cpp
Clang :: CXX/expr/expr.const/p2-0x.cpp
Clang :: CXX/expr/expr.prim/expr.prim.lambda/expr.prim.lambda.capture/p17.cpp
Clang :: CodeGen/ffp-contract-fast-option.cpp
Clang :: CodeGenCXX/const-init-cxx1y.cpp
Clang :: CodeGenCXX/const-init-cxx2a.cpp
Clang :: CodeGenCXX/non-const-init-cxx2a.cpp
Clang :: Sema/warn-unreachable.c
Clang :: Sema/warn-unsequenced.c
Clang :: SemaCXX/constant-expression-cxx1y.cpp
Clang :: SemaCXX/constant-expression-cxx2a.cpp
Clang :: SemaCXX/decomposed-condition.cpp
Clang :: SemaCXX/integer-overflow.cpp
Clang :: SemaCXX/warn-unsequenced.cpp

The new revision corrects all these fails. i have a couple questions about test cases, i've inline those in the test case modification. Also need to do clang-format and also some changes to CXXCallOperator to remove redundant accessor

clang/include/clang/AST/ExprCXX.h

168

i can get rid of these 2 accessor functions, we get them from CallExpr

clang/test/Analysis/loopexit-cfg-output.cpp

211 ↗

(On Diff #251699)

With these changes, the blocks print out in a different order but the semantic meaning appears to be the same. Is this difference acceptable?

clang/test/Sema/warn-unreachable.c

86 ↗

(On Diff #251699)

The srcpos for compound assignment is now the same as srcpos for ordinary binary operator, so i changed the test case to move the diagnostic, ok?

[mibintc]

This revision fixes the clang-tidy,clang-format and removes the redundant accessors. Ready for your review.

[rjmccall]

Let's make this patch be purely a representation change. I wouldn't expect *any* test changes from that; if there are, we should understand why. You can then add the stuff about tracking whether there's a pragma in a separate patch.

clang/include/clang/AST/StmtVisitor.h

92 ↗

(On Diff #251746)

Let's stick with the existing formatting for consistency. It's just vertically aligning the DISPATCH and class-name tokens to emphasize the common pattern.

clang/include/clang/Serialization/ASTBitCodes.h

1560 ↗

(On Diff #251746)

I think the intent is that we don't change constant values in this file. You can leave this in place with a comment saying it's defunct, though.

[mibintc]

In D76384#1934785, @rjmccall wrote:

Let's make this patch be purely a representation change. I wouldn't expect *any* test changes from that; if there are, we should understand why. You can then add the stuff about tracking whether there's a pragma in a separate patch.

OK, some of the tests show ConditionalOperator in the IR matching. Since that operator is necessarily getting merged into BinaryOperator those tests will have to change. I'll see about changing the srcpos for the conditional assignment operator so i can undo the srcpos changes in those tests.

[rjmccall]

In D76384#1936769, @mibintc wrote:

In D76384#1934785, @rjmccall wrote:

Let's make this patch be purely a representation change. I wouldn't expect *any* test changes from that; if there are, we should understand why. You can then add the stuff about tracking whether there's a pragma in a separate patch.

OK, some of the tests show ConditionalOperator in the IR matching. Since that operator is necessarily getting merged into BinaryOperator those tests will have to change. I'll see about changing the srcpos for the conditional assignment operator so i can undo the srcpos changes in those tests.

I assume you mean CompoundAssignOperator. Sure, if there's some sort of debug output / AST dump in a test that includes expression node names, obviously that's okay to change; and it's possible that the new behavior with CompoundAssignOperator not split out is better in some way. We should just make sure we understand any changes we see.

[mibintc]

I think this addresses all the feedback from @rjmccall

[mibintc]

This comment was removed by mibintc.

[mibintc]

I don't see those Harbormaster fails, using trunk and RHEL8 on architecture x86_64, Debug build of clang;clang-tools-extra and check-all. I ran all the failed lit tests separately and each passed

[rjmccall]

clang/include/clang/AST/Expr.h

2644

Let's split the CallExpr changes out into a separate patch, so that we have one patch that's *purely* about unifying BinaryOperator and CompoundAssignOperator and putting FPOptions in trailing storage, and then another that's about adding FPOptions to CallExpr.

For that latter patch, CallExpr already has its own, hand-rolled concept of trailing storage which you should be able to emulate instead of unifying the hierarchy.

3473

Why does this use unsigned for the trailing storage of the FPOptions?

3475

Can these bits go in the bitfields?

3522

Okay, so this is *always* adding trailing storage. Is there a common value for FPOptions — something that corresponds to default settings, for example — that we could use to avoid needing storage in the common case?

Also, it would be useful to extract a function somewhere that you can use in all of these places for consistency, maybe something like this on FPOptions:

/// Does this FPOptions require trailing storage when stored in various AST nodes,
/// or can it be recreated using `defaultWithoutTrailingStorage`?
bool requiresTrailingStorage() const { return *this == defaultWithoutTrailingStorage(); }

/// Return the default value of FPOptions that's used when trailing storage isn't required.
static FPOptions defaultWithoutTrailingStorage() { ... }

3528

Please change the text in this assertion and the one in the constructor above. The requirement is now that you use the appropriate constructor for the case. Or maybe we should just have one constructor that takes optional CompLHS/CompResult types and then assert that they're given if we're building a compound assignment? If you do the same for Create, the whole thing might end up being much more convenient for callers, too.

3699

Are these necessary?

3710

These should all assert that they're being used on a compound-assignment operator.

Please preserve the comments from CompoundAssignOperator on these.

6116

Why is this in this patch?

clang/lib/AST/Expr.cpp

4365

These places should use that same unified predicate.

[mibintc]

clang/include/clang/AST/Expr.h

3522

The reason I set hasFPFeatures to True in this revision is because in the previous review you asked me to make it purely a representational change and "add stuff about whether there is a pragma in a separate patch". The stuff I had in the previous version was smarter about creating trailing storage, it only created trailing storage when the pragma was in effect. I envirsioned that the evolution would accept something that always created the FPOptions in trailing storage, and in the 2nd generation would adopt the more thrifty way of creating trailing storage.

[rjmccall]

clang/include/clang/AST/Expr.h

3522

Well, let's at least set up the infrastructure and APIs correctly, even if we always allocate trailing storage.

For example, should getFPOptions take an ASTContext & so that it can always return the right options instead of making clients do hasFPOptions() ? getFPOptions() : ctx.getFPOptions()?

[mibintc]

@rjmccall Many thanks for your code review. Please take a look at this when you get a chance.

[mibintc]

I added some inline replies to John. I'm not certain I have everything yet exactly the way he wants it.

clang/include/clang/AST/Expr.h

2644

I've split off the CallExpr changes to a future patch - will you give me a hint about CallExpr's own hand-rolled concept of trailing storage? e.g. a line number or identifier name that i can hunt for and track down.

3473

I will change it to FPOptions

3474

I think I need hasFPFeatures in the node itself because that's how i know whether there's trailingstorage e.g. in the AST reader-writer. hasFPFeatures is always true at the moment but that will be improved in the next generation.

3475

i eliminated isCompoundAssign because, i can just check the opcode, i think there's always a opcode or a dummy opcode. But I need hasFPFeatures

3522

To explain further, in the pragma patch, which is fully formed but split off for a future commit, the Sema structure holds the current value of the floating point settings (in FPOptions FPFeatures). It is initialized from the command line, and as compound statements/blocks which contain pragma's are entered and exited, the value of Sema.FPFeatures is updated. I will add a bit "has_pragma_features" to FPOptions. When that bit is true, I know that the initial value of FPOptions (which is derived from command line) is different than the current settings. In this circumstance only it is necessary to hold FPFeatures in the Expr nodes (in Trailing storage). In all other cases, FPOptions can be derived from LangOpts and LangOpts is available in each clang pass. There are a bunch of occurrences where FPOptions() has been added as an nth parameter to various function calls, i believe though I'm not certain that in those cases the value of FPOptions is "don't care". (those occurrences were pre-existing in the source code). I hope this makes sense.

3528

I combined the dual Constructor's and Create's into singles with default parameters. I had to do some juggling at some of the call sites to make the new interface work. Do you prefer this way?

6116

I'm not sure what this is referring to?

clang/include/clang/AST/StmtVisitor.h

92 ↗

(On Diff #251746)

You had mentioned in your previous review about having things lined up properly here, and I don't disagree--this comment is just to let you know that I had applied the clang-format patch that was supplied by the BuildBots in the pre-commit checks. The buildbot pre-commit check wants them on different lines. I've switched the formatting to what you request.

clang/include/clang/Basic/LangOptions.h

405

I added these at your suggestion but not presently making use of them

clang/lib/AST/Expr.cpp

4365

I'm not sure if I captured what you want here.

[mibintc]

On my RHEL8 system building Debug clang;clang-tools-extra, check-all is passing.

[rjmccall]

clang/include/clang/AST/Expr.h

2644

Sure, check out CallExpr's sizeOfTrailingObjects and offsetToTrailingObjects.

3474

It's pretty common to make ASTStmtReader and ASTStmtWriter friends of the bitfields struct.

3475

If IsCompoundAssign is important enough to be worth optimizing, it's not a problem per se to keep it separate. Up to you. But it can definitely go in the bitfields if you need it.

3522

That does make sense, thank you. I wonder if it would be sensible for Sema to maintain not just the current state + whether there are any pragma overrides but also an idea of what specifically has been overridden. That would make the AST basically agnostic to the default settings; you'd always be able to just apply the overrides to the defaults. At that point, we'd be able to re-use serialized modules in builds with different fast-math settings, which would be really nice.

On the other hand, since the default settings are sometimes exposed as #defines (e.g. FAST_MATH) and therefore can influence the AST in other ways, maybe we wouldn't be able to re-use serialized modules that way.

Regardless, the right API would be to pass in the ASTContext so that this accessor can simply return the correct settings instead of making the caller merge information.

[rjmccall]

clang/include/clang/AST/Expr.h

3528

QualType has an empty state, so I would just declare the parameters like QualType CompLHSType = QualType(). That'll make the call sites much simpler, because you can structure the code like:

QualType CompLHSType, CompResultType;
if (E->isCompoundAssign()) {
  CompLHSType = ...;
  CompResultType = ...;
}
return BinaryOperator::Create(...);

3689

We generally lower-case method names, so these would be isCompoundAssign() and hasFPFeatures() (maybe hasFPFeatureOverrides()?).

6116

I was probably looking at the wrong diff, sorry. The "new changes only" display can get confused when patches are rebased.

clang/include/clang/Basic/LangOptions.h

405

Please do. You don't have to do it how I suggested — if you want to make requiresTrailingStorage just return true, that's fine — but please use this to centralize all the decisions about whether or not trailing storage is required.

[mibintc]

Here's another revision, I believe this handles all of @rjmccall 's requests. Thank you, John, for your review!

[rjmccall]

clang/include/clang/AST/Expr.h

3478

Sorry, I wasn't trying to say you need this here! Since you can use TrailingObjects for BinaryOperator, you absolutely should take advantage of what it does. I was just pointing you at the CallExpr stuff so that you can see the places you'll need to update when you add this storage to CallExpr.

3681

I'm not sure it's a good idea to have this accessor, at least not with such a tempting name. Maybe getStoredFPFeatures() and hasStoredFPFeatures()?

3695–3697

These two accessors will need to take ASTContexts eventually. Are you planning to do that in a follow-up patch?

clang/include/clang/AST/JSONNodeDumper.h

265 ↗

(On Diff #253983)

What actually calls this? StmtVisitor as written doesn't fall back on a VisitCompoundAssignOperator. You could *make* it fall back on one, of course.

clang/include/clang/AST/RecursiveASTVisitor.h

427 ↗

(On Diff #253983)

Comment needs updating.

clang/include/clang/AST/StmtVisitor.h

146 ↗

(On Diff #253983)

Comment needs updating, but more importantly, you're making all of these fall back on VisitBinAssign, which is definitely not correct.

clang/include/clang/Basic/LangOptions.h

418

The problem with having both functions that take ASTContexts and functions that don't is that it's easy to mix them, so they either need to have the same behavior (in which case it's pointless to have an overload that takes the ASTContext) or you're making something really error-prone.

I would feel a lot more confident that you were designing and using these APIs correctly if you actually took advantage of the ability to not store trailing FPOptions in some case, like when they match the global settings in the ASTContext. That way you'll actually be verifying that everything behaves correctly if nodes don't store FPOptions. If you do that, I think you'll see my point about not having all these easily-confusable functions that do or do not take ASTContexts..

[erichkeane]

Trying to help Melanie respond to the comments, so I ran through the patch and came up with the following comments/responses. Sorry for the 'surprise hit' :)

clang/include/clang/AST/Expr.h

3478

Yep, this shouldn't be necessary. Uses of this should be able to use totalSizeToAlloc and additionalSizeToAlloc.

3682

Whats the purpose of having both of these? It seems that you can either: 1- Have only the first and require consumers to check if they aren't sure, or 2- Only do the second, and those who have checked just get the default anyway.

3694

Since changing this value can result in a change of allocation size, I don't think this should be settable after creation.

3775

Nit, I'd prefer this be named 'isCompoundAssignment'. Also, the patch seems a little inconsistent with FPFeatures vs FPOptions.

clang/include/clang/AST/StmtVisitor.h

146 ↗

(On Diff #253983)

@rjmccall : What would you think fits better? If we don't have something like this, VisitBinaryOperator is going to have to redirect to VisitBinAssign anyway. We could presumably keep VisitCompoundAssignOperator, but that seems like a naming difference.

clang/include/clang/Basic/LangOptions.h

409

Nit: return {}; ?

Also, why are there two versions of this? It seems that the ASTContext one should be the only required one.

418

I think I disagree with @rjmccall that these requiresTrailingStorage should be here at all. I think we should store in the AST ANY programmer opinion, even if they match the global setting. It seems to me that this would be more tolerant of any global-setting rewrites that modules/et-al introduce, as well as make the AST Print consistent. Always storing FPOptions when the user has explicitly overriding it also better captures the programmer's intent.

clang/lib/AST/Expr.cpp

4354

Again, should just use the TrailingObjects::totalSizeToAlloc I think. Alternatively, implement a new 'totalSizeToAlloc' based on the hasFPFeatures and isCompound).

clang/lib/AST/ExprConstant.cpp

7788 ↗

(On Diff #253983)

Perhaps not changing this name is a good idea, and just leaving this as VisitCompoundOperator, then having the 'default' behavior to be to just call VisitBinaryOperator.

[rjmccall]

clang/include/clang/AST/StmtVisitor.h

146 ↗

(On Diff #253983)

VisitBinAssign is an existing method that is used specifically for the simple assignment operator. The old delegation was that VisitBinMulAssign delegated to VisitCompoundAssignOperator, which delegated to VisitBinaryOperator (because CAO was a subclass of that), which delegated to VisitExpr. The natural new delegation here would be for VisitBinMulAssign to just delegate directly to VisitBinaryOperator. If it's still useful to have an intermediate visitor method in the delegation chain for all the compound assignment operators, that's fine, but it shouldn't be VisitBinAssign.

clang/include/clang/Basic/LangOptions.h

418

I covered this elsewhere in the review. If you want to have that tolerance — and I think you should — then expressions should store (and Sema should track) the active pragma state, which can be most easily expressed as a pair of an FPOptions and a mask to apply to the global FPOptions. When you enter a pragma, you clear the relevant bits from the global FPOptions mask.

But the whole point of putting this stuff in trailing storage is so that you can make FPOptions as big as you need without actually inflating the AST size for a million nodes that don't care in the slightest about FPOptions.

[rjmccall]

clang/include/clang/Basic/LangOptions.h

418

But the whole point of putting this stuff in trailing storage is so that you can make FPOptions as big as you need without actually inflating the AST size for a million nodes that don't care in the slightest about FPOptions.

I meant to say: for a million nodes that don't care in the slightest about FPOptions, as well as for a million more nodes that aren't using pragma overrides.

[erichkeane]

clang/include/clang/Basic/LangOptions.h

418

Right, I get the intent, and I completely agree with that. My point was EVERY Expr that is affected by a #pragma should store it. Though, after looking at your Macro concern above, I'm less compelled.

I guess was suggesting that the logic for "requiresTrailingStorage" should just be "modified by a pragma" instead of "FPOptions != The global setting".

[rjmccall]

clang/include/clang/Basic/LangOptions.h

418

Well, "modified by a pragma" still wouldn't make the AST agnostic to global settings, since the pragmas don't override everything in FPOptions at once. But I agree that would achieve the most important goal, which is to stop inflating the AST when pragmas *aren't* in effect, which is approximately 100% of the time. In order to do that, though, we'll need to start tracking pragmas, which we should do but which can wait for a follow-up patch. In the meantime, I don't think you're ever going to get the interfaces right for queries like BinaryOperator::getFPOptions unless you actually stop relying on the fact that you're unconditionally storing FPOptions. You need to passing around ASTContexts for that. That's why I'm suggesting using an exact match with the global settings as a simple thing you can easily check without modifying what data you collect in FPOptions.

[erichkeane]

clang/include/clang/Basic/LangOptions.h

418

That sounds like a good plan to me. Thanks for entertaining my conversation/questions.

[sepavloff]

clang/include/clang/Basic/LangOptions.h

418

we'll need to start tracking pragmas

This is made in D76599 by representing floating point pragmas with a special statement node. These nodes allow an AST consumer like CodeGen or constant evaluator to maintain current set of floating options when it traverses AST. This approach looks simpler and more consistent as global state is represented as a variable in AST consumer and is not replicated to every relevant node. It makes easier to implement codegen for things like rounding mode, when change of the FP state requires specific instructions. A pragma statement can be used to generate required code. But if the state is spread by several nodes, it is more difficult for codegen to create necessary prolog/epilog code. Now compiler does not have support of properties that need synchronization with hardware, so these problems are not topical yet, but they eventually will arise.

[rjmccall]

clang/include/clang/Basic/LangOptions.h

418

Constant evaluation does not normally traverse the AST in the way you mean. It does this when evaluating a constexpr function, but that's not the dominant case of constant evaluation.

At the LLVM level, I think inlining, reordering, and ABI requirements on calls argue against a simple implementation model based on setting hardware flags when a pragma is entered and resetting them on exit.

[sepavloff]

clang/include/clang/Basic/LangOptions.h

418

Constant evaluation does not normally traverse the AST in the way you mean. It does this when evaluating a constexpr function, but that's not the dominant case of constant evaluation.

Evaluate* functions accept EvalInfo as argument, it can be extended to contain the current FPOptions, taken from Sema.

At the LLVM level, I think inlining, reordering, and ABI requirements on calls argue against a simple implementation model based on setting hardware flags when a pragma is entered and resetting them on exit.

For targets that encode FP environment in instructions this is true. But most targets encode FP environment in hardware registers, and a model, in which required FP environment is set at entry to some region and reset on exit from it, is very attractive. Actually constrained intrinsics is a way to prevent from reordering and similar optimizations that break this simple model. As C language provide setting FP environment only at block (or global) level it would be natural if AST would have similar property.

[rjmccall]

clang/include/clang/Basic/LangOptions.h

418

Many clients of the constant evaluator are not tied to Sema or are analyzing code that wasn't necessarily written in the current context. What you're discussing is *extremely* error-prone.

For targets that encode FP environment in instructions this is true. But most targets encode FP environment in hardware registers, and a model, in which required FP environment is set at entry to some region and reset on exit from it, is very attractive.

The constrained-intrinsics representation records options on each intrinsic, so no, it wouldn't naturally support a setup where the frontend emits intrinsics that change hardware flags on entry/exit to a region. That nicely matches a model where options are set on each expression. It also, fortunately, naturally supports optimizations like inlining as long as we turn non-intrinsic operations into intrinsics where necessary.

[mibintc]

I beleive this patch responds to all @rjmccall 's review comments, except I don't know how to program a solution to his StmtVisitor remark. I'll add more info about that. This patch optionally allocates the trailing storage in BinaryOperator. I verified that it was optionally allocated by adding an assert when trailing storage was allocated; the only lit tests that failed were the floating point pragma tests.

[mibintc]

If I change StmtVisitor to send compound assignment opcodes to BinaryOperator, then 12 clang LIT tests fail, all related to compound assignment mishandling, i.e. this patch

• a/clang/include/clang/AST/StmtVisitor.h

+++ b/clang/include/clang/AST/StmtVisitor.h
@@ -143,7 +143,7 @@ public:

// methods, fall back on VisitCompoundAssignOperator.

#define CAO_FALLBACK(NAME) \

RetTy VisitBin##NAME(PTR(BinaryOperator) S, ParamTys... P) {                 \

• DISPATCH(BinAssign, BinaryOperator); \

+ DISPATCH(BinaryOperator, BinaryOperator); \

}
CAO_FALLBACK(MulAssign) CAO_FALLBACK(DivAssign) CAO_FALLBACK(RemAssign)
CAO_FALLBACK(AddAssign) CAO_FALLBACK(SubAssign) CAO_FALLBACK(ShlAssign)

These clang lit tests fail,
Failing Tests (12):

Clang :: CXX/drs/dr2xx.cpp
Clang :: CXX/expr/expr.const/p2-0x.cpp
Clang :: CXX/expr/expr.prim/expr.prim.lambda/expr.prim.lambda.capture/p17.cpp
Clang :: CodeGenCXX/const-init-cxx1y.cpp
Clang :: CodeGenCXX/const-init-cxx2a.cpp
Clang :: CodeGenCXX/non-const-init-cxx2a.cpp
Clang :: Sema/warn-unsequenced.c
Clang :: SemaCXX/constant-expression-cxx1y.cpp
Clang :: SemaCXX/constant-expression-cxx2a.cpp
Clang :: SemaCXX/decomposed-condition.cpp
Clang :: SemaCXX/integer-overflow.cpp
Clang :: SemaCXX/warn-unsequenced.cpp

[sepavloff]

clang/include/clang/Basic/LangOptions.h

418

Many clients of the constant evaluator are not tied to Sema or are analyzing code that wasn't necessarily written in the current context. What you're discussing is *extremely* error-prone.

There is another implementation of representing FP environment in AST: D77545. Although it is made for limited number of cases, it could be extended.

[mibintc]

It seems like the macro's in StmtVisitor.h only work if CAO and BinaryOperator are separate classes.

[rjmccall]

I would guess that some visitor still has a VisitCompoundAssignOperator implementation, and that the behavior of VisitBinAssign happens to do what's needed for it.

You basically have two options:

• Keep a VisitCompoundAssignOperator around in StmtVisitor for all the compound-assignment operators to delegate to. You'll have to implement its delegation to VisitBinaryOperator manually.
• Search the source base for any remaining implementations of VisitCompoundAssignOperator and fold them into VisitBinaryOperator.

[mibintc]

I finally decided that combining BinaryOperator and CompoundAssignOperator was too difficult, this patch uses the trailing object approach similar to that used in CallExpr. @rjmccall thank you once again for all your reviews in developing this patch. I hope that this version is acceptable or within a hair's breadth of acceptable. As an experiment to confirm to myself that the trailing storage was only active in the pragma case, i added a "assert(0)" to getStoredFPFeatures. In that case check-clang showed 5 expected fails for the pragma tests cases. ( Clang :: CodeGen/constrained-math-builtins.c Clang :: CodeGen/fp-contract-fast-pragma.cpp Clang :: CodeGen/fp-contract-on-asm.c Clang :: CodeGen/fp-contract-on-pragma.cpp Clang :: CodeGen/fp-contract-pragma.cpp)

Then i removed that assert before submitting this to review.

[rjmccall]

Okay, that's fine, it's definitely a simpler change to not mess with the basic node hierarchy. The one problem I see is that you've (accidentally?) set up the FPOptions methods so that you can't call them on a BinaryOperator that's actually a CompoundAssignOperator — they'll end up accessing the wrong memory. Fortunately, that's pretty easy to fix, and fixing it should let you drop all the duplicate methods you've currently got on CompoundAssignOperator.

clang/include/clang/AST/Expr.h

3486

You should add an offsetOfTrailingStorage() method that returns isa<CompoundAssignOperator>(this) ? sizeof(CompoundAssignOperator) : sizeof(BinaryOperator), and then you can add that here instead of hard-coding sizeof(BinaryOperator).

[mibintc]

I made the changes requested by @rjmccall ; I also used clang-format on the tip. check-clang is passing. Look OK?

[rjmccall]

Thanks. Just a bunch of fairly minor requests now.

clang/include/clang/AST/Expr.h

3480

You don't need the non-const overload.

3723

Should HasFPFeatures be a bool everywhere you're passing it around?

clang/lib/AST/Expr.cpp

4402

Please define this in the header. You can just do:

inline size_t BinaryOperator::offsetOfTrailingStorage() const { ... }

after both of the classes are fully defined.

clang/lib/Analysis/BodyFarm.cpp

120

These construction sites don't seem like appropriate uses of defaultWithoutTrailingStorage, which is an implementation detail of the AST nodes. The right behavior here is for all of these places to use the default FPOptions from the language options, then let the AST figure out the right way to store that. That happens to have the same effect currently as defaultWithoutTrailingStorage, but the latter should be able to change, while the right behavior for these places will remain the same.

[mibintc]

Responded to @rjmccall 's review

[mibintc]

Adding an inline reply for John. rebased the patch, also re-applied clang-format. What do you think?

clang/lib/Analysis/BodyFarm.cpp

120

OK i changed these in BodyFarm to use the default constructor. In Sema I changed them from defaultWithoutTrailingStorage to use Sema.FPFeatures which is the current setting combining pragma values + command line options.

[rjmccall]

clang/lib/Analysis/BodyFarm.cpp

120

That's right for Sema. For all the other generated uses, I think you should use FPOptions(C.getLangOpts()) instead of the default constructor. Most of these don't really care about FPOptions — they're just creating an assignment or some other operation which is not FP-sensitive — but for the ones that do, we should follow the default behavior in the language options.

[mibintc]

Lost power in Monday's storm, back online today. I made the changes requested by @rjmccall. Look OK?

[rjmccall]

Minor fixes, then LGTM.

clang/lib/AST/ASTImporter.cpp

6822

Oh, these two calls need to use E->getFPFeatures(Importer.getFromContext()), since that's the context that the node was built in.

clang/lib/CodeGen/CGObjC.cpp

1497

Missed this one.

clang/lib/CodeGen/CGStmtOpenMP.cpp

2935

Missed this one.

clang/lib/Sema/SemaOverload.cpp

13784

This should use FPFeatures, I think.

[mibintc]

Responding to @rjmccall 's review. John, after this is approved I want to proceed with pragma float_control as proposed in D72841. Can you recommend an approach, do you think I will need to do it incrementally?

[rjmccall]

In D76384#1984498, @mibintc wrote:

Responding to @rjmccall 's review. John, after this is approved I want to proceed with pragma float_control as proposed in D72841. Can you recommend an approach, do you think I will need to do it incrementally?

I think the basic technical approach from that patch seems reasonable. It's not my goal to hold up your work to pursue other technical goals; we took this detour just to minimize the memory impact of your work.

I *would* like an NFC patch first that renames FPFeatures to something like CurFPFeatures in order to more clearly distinguish it from FPOptions in Sema code, though. That should be very quick.

[rjmccall]

Oh, and this patch LGTM, thank you.

[mibintc]

In D76384#1984584, @rjmccall wrote:

In D76384#1984498, @mibintc wrote:

Responding to @rjmccall 's review. John, after this is approved I want to proceed with pragma float_control as proposed in D72841. Can you recommend an approach, do you think I will need to do it incrementally?

I think the basic technical approach from that patch seems reasonable. It's not my goal to hold up your work to pursue other technical goals; we took this detour just to minimize the memory impact of your work.

Thanks for all. I'm sure my work will be easier in the long run to have this well scrutinized. And I learned a lot. And I endorse not being wasteful of memory.

I *would* like an NFC patch first that renames FPFeatures to something like CurFPFeatures in order to more clearly distinguish it from FPOptions in Sema code, though. That should be very quick.

OK. I think this means I can commit now and change the identifier spelling in a future standalone patch.

[mibintc]

In D76384#1984584, @rjmccall wrote:

I *would* like an NFC patch first that renames FPFeatures to something like CurFPFeatures in order to more clearly distinguish it from FPOptions in Sema code, though. That should be very quick.

You mean, Sema.FPFeatures -> Sema.CurFPFeatures, confirm?

[rjmccall]

In D76384#1984683, @mibintc wrote:

In D76384#1984584, @rjmccall wrote:

I *would* like an NFC patch first that renames FPFeatures to something like CurFPFeatures in order to more clearly distinguish it from FPOptions in Sema code, though. That should be very quick.

You mean, Sema.FPFeatures -> Sema.CurFPFeatures, confirm?

Right.

[mibintc]

@rjmccall Can you check the patch added last night here, commit 3ee1ec0b9dd6ee2350f39ae8a418bf3ce28d06cf
Author: Benjamin Kramer <[e-mail]>
Date: Thu Apr 16 11:45:02 2020 +0200

LangOptions cannot depend on ASTContext, make it not use ASTContext directly

Fixes a layering violation introduced in 2ba4e3a4598b165245c581c506a813cd4a7dce33.

[martong]

clang/lib/AST/ASTImporter.cpp

6821

This introduced an assertion failure during CTU analysis. The reason is that the LHSType and the ResultType have been imported twice.

The fix is in e033ec291a1b72f307ab14569ca99822c127610b

Details:

   clang: ../../git/llvm-project/clang/lib/Basic/SourceManager.cpp:918: clang::FileID clang::SourceManager::getFileIDLoaded(unsigned int) const: Assertion `0 && "Invalid SLocOffset or bad function choice"' failed.
   clang::SourceManager::getDecomposedExpansionLoc(clang::SourceLocation) const
   clang::SourceManager::getPresumedLoc(clang::SourceLocation, bool) const
   clang::ASTImporter::Import(clang::SourceLocation)
   llvm::Error clang::ASTImporter::importInto<clang::SourceLocation>(clang::SourceLocation&, clang::SourceLocation const&)
   clang::ASTNodeImporter::ImportDeclParts(clang::NamedDecl*, clang::DeclContext*&, clang::DeclContext*&, clang::DeclarationName&, clang::NamedDecl*&, clang::SourceLocation&)
   clang::ASTNodeImporter::VisitRecordDecl(clang::RecordDecl*)
   clang::declvisitor::Base<std::add_pointer, clang::ASTNodeImporter, llvm::Expected<clang::Decl*> >::Visit(clang::Decl*)
   clang::ASTImporter::Import(clang::Decl*)
   clang::ASTNodeImporter::VisitRecordType(clang::RecordType const*)
   clang::TypeVisitor<clang::ASTNodeImporter, llvm::Expected<clang::QualType> >::Visit(clang::Type const*)
   clang::ASTImporter::Import(clang::QualType)
   clang::ASTNodeImporter::VisitElaboratedType(clang::ElaboratedType const*)
   clang::TypeVisitor<clang::ASTNodeImporter, llvm::Expected<clang::QualType> >::Visit(clang::Type const*)
   clang::ASTImporter::Import(clang::QualType)
   clang::ASTNodeImporter::VisitPointerType(clang::PointerType const*)
   clang::TypeVisitor<clang::ASTNodeImporter, llvm::Expected<clang::QualType> >::Visit(clang::Type const*)
   clang::ASTImporter::Import(clang::QualType)
   clang::QualType clang::ASTNodeImporter::importChecked<clang::QualType>(llvm::Error&, clang::QualType const&)
 
clang::ASTNodeImporter::VisitCompoundAssignOperator(clang::CompoundAssignOperator*)

[mibintc]

clang/lib/AST/ASTImporter.cpp

6821

@martong Thank you.

[mibintc]

In D76384#1986525, @mibintc wrote:

@rjmccall Can you check the patch added last night here, commit 3ee1ec0b9dd6ee2350f39ae8a418bf3ce28d06cf
Author: Benjamin Kramer <[e-mail]>
Date: Thu Apr 16 11:45:02 2020 +0200

LangOptions cannot depend on ASTContext, make it not use ASTContext directly

Fixes a layering violation introduced in 2ba4e3a4598b165245c581c506a813cd4a7dce33.

I checked Benny's patch and it seems to have all the right semantics. The lit tests pass and the trailing storage is only created in the pragma case.

[mibintc]

I also added this patch which is a companion to this.
commit 8812b0cc5cc09f350d8e89bff99f185c5e1a5d4d
Author: Melanie Blower <[e-mail]>
Date: Thu Apr 16 08:45:26 2020 -0700

[NFC] Rename Sema.FPFeatures to CurFPFeatures and accessor to getCurFPFeatures

[rjmccall]

In D76384#1986761, @mibintc wrote:

In D76384#1986525, @mibintc wrote:

@rjmccall Can you check the patch added last night here, commit 3ee1ec0b9dd6ee2350f39ae8a418bf3ce28d06cf
Author: Benjamin Kramer <[e-mail]>
Date: Thu Apr 16 11:45:02 2020 +0200

LangOptions cannot depend on ASTContext, make it not use ASTContext directly

Fixes a layering violation introduced in 2ba4e3a4598b165245c581c506a813cd4a7dce33.

I checked Benny's patch and it seems to have all the right semantics. The lit tests pass and the trailing storage is only created in the pragma case.

Looks good to me, too.

clang/include/clang/AST/Expr.h

class CallExpr : public Expr {
   /// this pointer to the trailing objects.
   static unsigned offsetToTrailingObjects(StmtClass SC);
 
 public:
   enum class ADLCallKind : bool { NotADL, UsesADL };
   static constexpr ADLCallKind NotADL = ADLCallKind::NotADL;
   static constexpr ADLCallKind UsesADL = ADLCallKind::UsesADL;
 
 protected:
   /// Build a call expression, assuming that appropriate storage has been
   /// allocated for the trailing objects.
   CallExpr(StmtClass SC, Expr *Fn, ArrayRef<Expr *> PreArgs,
            ArrayRef<Expr *> Args, QualType Ty, ExprValueKind VK,
            SourceLocation RParenLoc, unsigned MinNumArgs, ADLCallKind UsesADL);
 
   /// Build an empty call expression, for deserialization.
   CallExpr(StmtClass SC, unsigned NumPreArgs, unsigned NumArgs,

 /// for x and y. If neither x or y is type-dependent, and the "+"
 /// operator resolves to a built-in operation, BinaryOperator will be
 /// used to express the computation (x and y may still be
 /// value-dependent). If either x or y is type-dependent, or if the
 /// "+" resolves to an overloaded operator, CXXOperatorCallExpr will
 /// be used to express the computation.
 class BinaryOperator : public Expr {
   enum { LHS, RHS, END_EXPR };
   Stmt *SubExprs[END_EXPR];
 
 public:
   typedef BinaryOperatorKind Opcode;
 
-  BinaryOperator(Expr *lhs, Expr *rhs, Opcode opc, QualType ResTy,
-                 ExprValueKind VK, ExprObjectKind OK, SourceLocation opLoc,
-                 FPOptions FPFeatures)
+protected:
+  size_t offsetOfTrailingStorage() const;
+
+  /// Return a pointer to the trailing FPOptions
+  FPOptions *getTrailingFPFeatures() {
+    assert(BinaryOperatorBits.HasFPFeatures);
+    return reinterpret_cast<FPOptions *>(reinterpret_cast<char *>(this) +
+                                         offsetOfTrailingStorage());
+  }
+  const FPOptions *getTrailingFPFeatures() const {
+    assert(BinaryOperatorBits.HasFPFeatures);
+    return reinterpret_cast<const FPOptions *>(
+        reinterpret_cast<const char *>(this) + offsetOfTrailingStorage());
+  }
+
+  /// Build a binary operator, assuming that appropriate storage has been
+  /// allocated for the trailing objects when needed.
+  BinaryOperator(const ASTContext &Ctx, Expr *lhs, Expr *rhs, Opcode opc,
+                 QualType ResTy, ExprValueKind VK, ExprObjectKind OK,
+                 SourceLocation opLoc, FPOptions FPFeatures)
       : Expr(BinaryOperatorClass, ResTy, VK, OK) {
     BinaryOperatorBits.Opc = opc;
-    BinaryOperatorBits.FPFeatures = FPFeatures.getInt();
+    assert(!isCompoundAssignmentOp() &&
+           "Use CompoundAssignOperator for compound assignments");
     BinaryOperatorBits.OpLoc = opLoc;
     SubExprs[LHS] = lhs;
     SubExprs[RHS] = rhs;
-    assert(!isCompoundAssignmentOp() &&
-           "Use CompoundAssignOperator for compound assignments");
+    BinaryOperatorBits.HasFPFeatures = FPFeatures.requiresTrailingStorage(Ctx);
+    if (BinaryOperatorBits.HasFPFeatures)
+      *getTrailingFPFeatures() = FPFeatures;
     setDependence(computeDependence(this));
   }
 
   /// Construct an empty binary operator.
   explicit BinaryOperator(EmptyShell Empty) : Expr(BinaryOperatorClass, Empty) {
     BinaryOperatorBits.Opc = BO_Comma;
   }
 
+public:
+  static BinaryOperator *CreateEmpty(const ASTContext &C, bool hasFPFeatures);
+
+  static BinaryOperator *Create(const ASTContext &C, Expr *lhs, Expr *rhs,
+                                Opcode opc, QualType ResTy, ExprValueKind VK,
+                                ExprObjectKind OK, SourceLocation opLoc,
+                                FPOptions FPFeatures);
   SourceLocation getExprLoc() const { return getOperatorLoc(); }
   SourceLocation getOperatorLoc() const { return BinaryOperatorBits.OpLoc; }
   void setOperatorLoc(SourceLocation L) { BinaryOperatorBits.OpLoc = L; }
 
   Opcode getOpcode() const {
     return static_cast<Opcode>(BinaryOperatorBits.Opc);
   }
   void setOpcode(Opcode Opc) { BinaryOperatorBits.Opc = Opc; }
 
   Expr *getLHS() const { return cast<Expr>(SubExprs[LHS]); }
   void setLHS(Expr *E) { SubExprs[LHS] = E; }
   Expr *getRHS() const { return cast<Expr>(SubExprs[RHS]); }
   void setRHS(Expr *E) { SubExprs[RHS] = E; }
 
public:
   // Iterators
   child_range children() {
     return child_range(&SubExprs[0], &SubExprs[0]+END_EXPR);
   }
   const_child_range children() const {
     return const_child_range(&SubExprs[0], &SubExprs[0] + END_EXPR);
   }
 
-  // Set the FP contractability status of this operator. Only meaningful for
-  // operations on floating point types.
-  void setFPFeatures(FPOptions F) {
-    BinaryOperatorBits.FPFeatures = F.getInt();
+  /// Set and fetch the bit that shows whether FPFeatures needs to be
+  /// allocated in Trailing Storage
+  void setHasStoredFPFeatures(bool B) { BinaryOperatorBits.HasFPFeatures = B; }
+  bool hasStoredFPFeatures() const { return BinaryOperatorBits.HasFPFeatures; }
+
+  /// Get FPFeatures from trailing storage
+  FPOptions getStoredFPFeatures() const {
+    assert(hasStoredFPFeatures());
+    return *getTrailingFPFeatures();
+  }
+  /// Set FPFeatures in trailing storage, used only by Serialization
+  void setStoredFPFeatures(FPOptions F) {
+    assert(BinaryOperatorBits.HasFPFeatures);
+    *getTrailingFPFeatures() = F;
   }
 
-  FPOptions getFPFeatures() const {
-    return FPOptions(BinaryOperatorBits.FPFeatures);
+  // Get the FP features status of this operator. Only meaningful for
+  // operations on floating point types.
+  FPOptions getFPFeatures(const ASTContext &C) const {
+    if (BinaryOperatorBits.HasFPFeatures)
+      return getStoredFPFeatures();
+    return FPOptions::defaultWithoutTrailingStorage(C);
   }
 
   // Get the FP contractability status of this operator. Only meaningful for
   // operations on floating point types.
-  bool isFPContractableWithinStatement() const {
-    return getFPFeatures().allowFPContractWithinStatement();
+  bool isFPContractableWithinStatement(const ASTContext &C) const {
+    return getFPFeatures(C).allowFPContractWithinStatement();
   }
 
   // Get the FENV_ACCESS status of this operator. Only meaningful for
   // operations on floating point types.
-  bool isFEnvAccessOn() const { return getFPFeatures().allowFEnvAccess(); }
+  bool isFEnvAccessOn(const ASTContext &C) const {
+    return getFPFeatures(C).allowFEnvAccess();
+  }
 
 protected:
-  BinaryOperator(Expr *lhs, Expr *rhs, Opcode opc, QualType ResTy,
-                 ExprValueKind VK, ExprObjectKind OK, SourceLocation opLoc,
-                 FPOptions FPFeatures, bool dead2)
+  BinaryOperator(const ASTContext &Ctx, Expr *lhs, Expr *rhs, Opcode opc,
+                 QualType ResTy, ExprValueKind VK, ExprObjectKind OK,
+                 SourceLocation opLoc, FPOptions FPFeatures, bool dead2)
       : Expr(CompoundAssignOperatorClass, ResTy, VK, OK) {
     BinaryOperatorBits.Opc = opc;
-    BinaryOperatorBits.FPFeatures = FPFeatures.getInt();
+    assert(isCompoundAssignmentOp() &&
+           "Use CompoundAssignOperator for compound assignments");
     BinaryOperatorBits.OpLoc = opLoc;
     SubExprs[LHS] = lhs;
     SubExprs[RHS] = rhs;
+    BinaryOperatorBits.HasFPFeatures = FPFeatures.requiresTrailingStorage(Ctx);
+    if (BinaryOperatorBits.HasFPFeatures)
+      *getTrailingFPFeatures() = FPFeatures;
     setDependence(computeDependence(this));
   }
 
+  /// Construct an empty BinaryOperator, SC is CompoundAssignOperator.
   BinaryOperator(StmtClass SC, EmptyShell Empty) : Expr(SC, Empty) {
     BinaryOperatorBits.Opc = BO_MulAssign;
   }
+
+  /// Return the size in bytes needed for the trailing objects.
+  /// Used to allocate the right amount of storage.
+  static unsigned sizeOfTrailingObjects(bool HasFPFeatures) {
+    return HasFPFeatures * sizeof(FPOptions);
+  }
 };
 
 /// CompoundAssignOperator - For compound assignments (e.g. +=), we keep
 /// track of the type the operation is performed in.  Due to the semantics of
 /// these operators, the operands are promoted, the arithmetic performed, an
 /// implicit conversion back to the result type done, then the assignment takes
 /// place.  This captures the intermediate type which the computation is done
 /// in.
 class CompoundAssignOperator : public BinaryOperator {
   QualType ComputationLHSType;
   QualType ComputationResultType;
-public:
-  CompoundAssignOperator(Expr *lhs, Expr *rhs, Opcode opc, QualType ResType,
-                         ExprValueKind VK, ExprObjectKind OK,
-                         QualType CompLHSType, QualType CompResultType,
-                         SourceLocation OpLoc, FPOptions FPFeatures)
-    : BinaryOperator(lhs, rhs, opc, ResType, VK, OK, OpLoc, FPFeatures,
+
+  /// Construct an empty CompoundAssignOperator.
+  explicit CompoundAssignOperator(const ASTContext &C, EmptyShell Empty,
+                                  bool hasFPFeatures)
+      : BinaryOperator(CompoundAssignOperatorClass, Empty) {}
+
+protected:
+  CompoundAssignOperator(const ASTContext &C, Expr *lhs, Expr *rhs, Opcode opc,
+                         QualType ResType, ExprValueKind VK, ExprObjectKind OK,
+                         SourceLocation OpLoc, FPOptions FPFeatures,
+                         QualType CompLHSType, QualType CompResultType)
+      : BinaryOperator(C, lhs, rhs, opc, ResType, VK, OK, OpLoc, FPFeatures,
                        true),
-      ComputationLHSType(CompLHSType),
-      ComputationResultType(CompResultType) {
+        ComputationLHSType(CompLHSType), ComputationResultType(CompResultType) {
     assert(isCompoundAssignmentOp() &&
            "Only should be used for compound assignments");
   }
 
-  /// Build an empty compound assignment operator expression.
-  explicit CompoundAssignOperator(EmptyShell Empty)
-    : BinaryOperator(CompoundAssignOperatorClass, Empty) { }
+public:
+  static CompoundAssignOperator *CreateEmpty(const ASTContext &C,
+                                             bool hasFPFeatures);
+
+  static CompoundAssignOperator *
+  Create(const ASTContext &C, Expr *lhs, Expr *rhs, Opcode opc, QualType ResTy,
+         ExprValueKind VK, ExprObjectKind OK, SourceLocation opLoc,
+         FPOptions FPFeatures, QualType CompLHSType = QualType(),
+         QualType CompResultType = QualType());
 
   // The two computation types are the type the LHS is converted
   // to for the computation and the type of the result; the two are
   // distinct in a few cases (specifically, int+=ptr and ptr-=ptr).
   QualType getComputationLHSType() const { return ComputationLHSType; }
   void setComputationLHSType(QualType T) { ComputationLHSType = T; }
 
   QualType getComputationResultType() const { return ComputationResultType; }
   void setComputationResultType(QualType T) { ComputationResultType = T; }
 
   static bool classof(const Stmt *S) {
     return S->getStmtClass() == CompoundAssignOperatorClass;
   }
 };
 
+inline size_t BinaryOperator::offsetOfTrailingStorage() const {
+  assert(BinaryOperatorBits.HasFPFeatures);
+  return isa<CompoundAssignOperator>(this) ? sizeof(CompoundAssignOperator)
+                                           : sizeof(BinaryOperator);
+}
+
 /// AbstractConditionalOperator - An abstract base class for
 /// ConditionalOperator and BinaryConditionalOperator.
 class AbstractConditionalOperator : public Expr {
   SourceLocation QuestionLoc, ColonLoc;
   friend class ASTStmtReader;
 
 protected:
   AbstractConditionalOperator(StmtClass SC, QualType T, ExprValueKind VK,
private:
 
   size_t numTrailingObjects(OverloadToken<Stmt *>) const { return NumExprs; }
 
   SourceLocation BeginLoc, EndLoc;
   unsigned NumExprs;
   friend TrailingObjects;
   friend class ASTStmtReader;
   friend class ASTStmtWriter;
 };
 
 } // end namespace clang
 
 #endif // LLVM_CLANG_AST_EXPR_H

clang/include/clang/AST/ExprCXX.h

public:
   SourceRange getSourceRange() const { return Range; }
 
   static bool classof(const Stmt *T) {
     return T->getStmtClass() == CXXOperatorCallExprClass;
   }
 
   // Set the FP contractability status of this operator. Only meaningful for
   // operations on floating point types.
   void setFPFeatures(FPOptions F) {
-    CXXOperatorCallExprBits.FPFeatures = F.getInt();
+    CXXOperatorCallExprBits.FPFeatures = F.getAsOpaqueInt();
   }
   FPOptions getFPFeatures() const {
     return FPOptions(CXXOperatorCallExprBits.FPFeatures);
   }
 
   // Get the FP contractability status of this operator. Only meaningful for
   // operations on floating point types.
   bool isFPContractableWithinStatement() const {

clang/include/clang/AST/Stmt.h

protected:
   class BinaryOperatorBitfields {
     friend class BinaryOperator;
 
     unsigned : NumExprBits;
 
     unsigned Opc : 6;
 
     /// This is only meaningful for operations on floating point
-    /// types and 0 otherwise.
-    unsigned FPFeatures : 8;
+    /// types when additional values need to be in trailing storage.
+    /// It is 0 otherwise.
+    unsigned HasFPFeatures : 1;
 
     SourceLocation OpLoc;
   };
 
   class InitListExprBitfields {
     friend class InitListExpr;
 
     unsigned : NumExprBits;

clang/include/clang/Basic/LangOptions.h

 #include "llvm/ADT/FloatingPointMode.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/Triple.h"
 #include <string>
 #include <vector>
 
 namespace clang {
 
+class ASTContext;
+
 /// Bitfields of LangOptions, split out from LangOptions in order to ensure that
 /// this large collection of bitfields is a trivial class type.
 class LangOptionsBase {
 public:
   // Define simple language options (with no accessors).
 #define LANGOPT(Name, Bits, Default, Description) unsigned Name : Bits;
 #define ENUM_LANGOPT(Name, Type, Bits, Default, Description)
 #include "clang/Basic/LangOptions.def"
public:
   explicit FPOptions(const LangOptions &LangOpts)
       : fp_contract(LangOpts.getDefaultFPContractMode()),
         fenv_access(LangOptions::FEA_Off),
         rounding(LangOptions::FPR_ToNearest),
         exceptions(LangOptions::FPE_Ignore)
         {}
   // FIXME: Use getDefaultFEnvAccessMode() when available.
 
+  /// Return the default value of FPOptions that's used when trailing
+  /// storage isn't required.
+  static FPOptions defaultWithoutTrailingStorage(const ASTContext &C);
+
+  /// Does this FPOptions require trailing storage when stored in various
+  /// AST nodes, or can it be recreated using `defaultWithoutTrailingStorage`?
+  bool requiresTrailingStorage(const ASTContext &C);
+
   bool allowFPContractWithinStatement() const {
     return fp_contract == LangOptions::FPC_On;
   }
 
   bool allowFPContractAcrossStatement() const {
     return fp_contract == LangOptions::FPC_Fast;
   }
 
   void setAllowFPContractWithinStatement() {
     fp_contract = LangOptions::FPC_On;
   }
 
   void setAllowFPContractAcrossStatement() {
     fp_contract = LangOptions::FPC_Fast;
   }
public:
 
   bool isFPConstrained() const {
     return getRoundingMode() != RoundingMode::NearestTiesToEven ||
            getExceptionMode() != LangOptions::FPE_Ignore ||
            allowFEnvAccess();
   }
 
   /// Used to serialize this.
-  unsigned getInt() const {
-    return fp_contract | (fenv_access << 2) | (rounding << 3)
-        | (exceptions << 6);
+  unsigned getAsOpaqueInt() const {
+    return fp_contract | (fenv_access << 2) | (rounding << 3) |
+           (exceptions << 6);
   }
 
 private:
   /// Adjust BinaryOperatorBitfields::FPFeatures and
   /// CXXOperatorCallExprBitfields::FPFeatures to match the total bit-field size
   /// of these fields.
   unsigned fp_contract : 2;
   unsigned fenv_access : 1;

clang/lib/AST/ASTImporter.cpp

ExpectedStmt ASTNodeImporter::VisitBinaryOperator(BinaryOperator *E) {
   Error Err = Error::success();
   auto ToLHS = importChecked(Err, E->getLHS());
   auto ToRHS = importChecked(Err, E->getRHS());
   auto ToType = importChecked(Err, E->getType());
   auto ToOperatorLoc = importChecked(Err, E->getOperatorLoc());
   if (Err)
     return std::move(Err);
 
-  return new (Importer.getToContext()) BinaryOperator(
-      ToLHS, ToRHS, E->getOpcode(), ToType, E->getValueKind(),
-      E->getObjectKind(), ToOperatorLoc, E->getFPFeatures());
+  return BinaryOperator::Create(Importer.getToContext(), ToLHS, ToRHS,
+                                E->getOpcode(), ToType, E->getValueKind(),
+                                E->getObjectKind(), ToOperatorLoc,
+                                E->getFPFeatures(Importer.getFromContext()));
 }
 
 ExpectedStmt ASTNodeImporter::VisitConditionalOperator(ConditionalOperator *E) {
   Error Err = Error::success();
   auto ToCond = importChecked(Err, E->getCond());
   auto ToQuestionLoc = importChecked(Err, E->getQuestionLoc());
   auto ToLHS = importChecked(Err, E->getLHS());
   auto ToColonLoc = importChecked(Err, E->getColonLoc());
ASTNodeImporter::VisitCompoundAssignOperator(CompoundAssignOperator *E) {
   auto ToType = importChecked(Err, E->getType());
   auto ToComputationLHSType = importChecked(Err, E->getComputationLHSType());
   auto ToComputationResultType =
       importChecked(Err, E->getComputationResultType());
   auto ToOperatorLoc = importChecked(Err, E->getOperatorLoc());
   if (Err)
     return std::move(Err);
 
-  return new (Importer.getToContext()) CompoundAssignOperator(
-      ToLHS, ToRHS, E->getOpcode(), ToType, E->getValueKind(),
-      E->getObjectKind(), ToComputationLHSType, ToComputationResultType,
-      ToOperatorLoc, E->getFPFeatures());
+  return CompoundAssignOperator::Create(
+      Importer.getToContext(), ToLHS, ToRHS, E->getOpcode(), ToType,
+      E->getValueKind(), E->getObjectKind(), ToOperatorLoc,
+      E->getFPFeatures(Importer.getFromContext()),
+      importChecked(Err, ToComputationLHSType),
+      importChecked(Err, ToComputationResultType));
 }
 
 Expected<CXXCastPath>
 ASTNodeImporter::ImportCastPath(CastExpr *CE) {
   CXXCastPath Path;
   for (auto I = CE->path_begin(), E = CE->path_end(); I != E; ++I) {
     if (auto SpecOrErr = import(*I))
       Path.push_back(*SpecOrErr);

clang/lib/AST/Expr.cpp

 
 ParenListExpr *ParenListExpr::CreateEmpty(const ASTContext &Ctx,
                                           unsigned NumExprs) {
   void *Mem =
       Ctx.Allocate(totalSizeToAlloc<Stmt *>(NumExprs), alignof(ParenListExpr));
   return new (Mem) ParenListExpr(EmptyShell(), NumExprs);
 }
 
+BinaryOperator *BinaryOperator::CreateEmpty(const ASTContext &C,
+                                            bool HasFPFeatures) {
+  unsigned Extra = sizeOfTrailingObjects(HasFPFeatures);
+  void *Mem =
+      C.Allocate(sizeof(BinaryOperator) + Extra, alignof(BinaryOperator));
+  return new (Mem) BinaryOperator(EmptyShell());
+}
+
+BinaryOperator *BinaryOperator::Create(const ASTContext &C, Expr *lhs,
+                                       Expr *rhs, Opcode opc, QualType ResTy,
+                                       ExprValueKind VK, ExprObjectKind OK,
+                                       SourceLocation opLoc,
+                                       FPOptions FPFeatures) {
+  bool HasFPFeatures = FPFeatures.requiresTrailingStorage(C);
+  unsigned Extra = sizeOfTrailingObjects(HasFPFeatures);
+  void *Mem =
+      C.Allocate(sizeof(BinaryOperator) + Extra, alignof(BinaryOperator));
+  return new (Mem)
+      BinaryOperator(C, lhs, rhs, opc, ResTy, VK, OK, opLoc, FPFeatures);
+}
+
+CompoundAssignOperator *
+CompoundAssignOperator::CreateEmpty(const ASTContext &C, bool HasFPFeatures) {
+  unsigned Extra = sizeOfTrailingObjects(HasFPFeatures);
+  void *Mem = C.Allocate(sizeof(CompoundAssignOperator) + Extra,
+                         alignof(CompoundAssignOperator));
+  return new (Mem) CompoundAssignOperator(C, EmptyShell(), HasFPFeatures);
+}
+
+CompoundAssignOperator *CompoundAssignOperator::Create(
+    const ASTContext &C, Expr *lhs, Expr *rhs, Opcode opc, QualType ResTy,
+    ExprValueKind VK, ExprObjectKind OK, SourceLocation opLoc,
+    FPOptions FPFeatures, QualType CompLHSType, QualType CompResultType) {
+  bool HasFPFeatures = FPFeatures.requiresTrailingStorage(C);
+  unsigned Extra = sizeOfTrailingObjects(HasFPFeatures);
+  void *Mem = C.Allocate(sizeof(CompoundAssignOperator) + Extra,
+                         alignof(CompoundAssignOperator));
+  return new (Mem)
+      CompoundAssignOperator(C, lhs, rhs, opc, ResTy, VK, OK, opLoc, FPFeatures,
+                             CompLHSType, CompResultType);
+}
+
 const OpaqueValueExpr *OpaqueValueExpr::findInCopyConstruct(const Expr *e) {
   if (const ExprWithCleanups *ewc = dyn_cast<ExprWithCleanups>(e))
     e = ewc->getSubExpr();
   if (const MaterializeTemporaryExpr *m = dyn_cast<MaterializeTemporaryExpr>(e))
     e = m->getSubExpr();
   e = cast<CXXConstructExpr>(e)->getArg(0);
   while (const ImplicitCastExpr *ice = dyn_cast<ImplicitCastExpr>(e))
     e = ice->getSubExpr();
   return cast<OpaqueValueExpr>(e);
 }
 
 PseudoObjectExpr *PseudoObjectExpr::Create(const ASTContext &Context,
                                            EmptyShell sh,
                                            unsigned numSemanticExprs) {
   void *buffer =
       Context.Allocate(totalSizeToAlloc<Expr *>(1 + numSemanticExprs),
                        alignof(PseudoObjectExpr));
   return new(buffer) PseudoObjectExpr(sh, numSemanticExprs);
 }

clang/lib/AST/ExprCXX.cpp

CXXOperatorCallExpr::CXXOperatorCallExpr(OverloadedOperatorKind OpKind,
                                          Expr *Fn, ArrayRef<Expr *> Args,
                                          QualType Ty, ExprValueKind VK,
                                          SourceLocation OperatorLoc,
                                          FPOptions FPFeatures,
                                          ADLCallKind UsesADL)
     : CallExpr(CXXOperatorCallExprClass, Fn, /*PreArgs=*/{}, Args, Ty, VK,
                OperatorLoc, /*MinNumArgs=*/0, UsesADL) {
   CXXOperatorCallExprBits.OperatorKind = OpKind;
-  CXXOperatorCallExprBits.FPFeatures = FPFeatures.getInt();
+  CXXOperatorCallExprBits.FPFeatures = FPFeatures.getAsOpaqueInt();
   assert(
       (CXXOperatorCallExprBits.OperatorKind == static_cast<unsigned>(OpKind)) &&
       "OperatorKind overflow!");
-  assert((CXXOperatorCallExprBits.FPFeatures == FPFeatures.getInt()) &&
+  assert((CXXOperatorCallExprBits.FPFeatures == FPFeatures.getAsOpaqueInt()) &&
          "FPFeatures overflow!");
   Range = getSourceRangeImpl();
 }
 
 CXXOperatorCallExpr::CXXOperatorCallExpr(unsigned NumArgs, EmptyShell Empty)
     : CallExpr(CXXOperatorCallExprClass, /*NumPreArgs=*/0, NumArgs, Empty) {}
 
 CXXOperatorCallExpr *CXXOperatorCallExpr::Create(
CUDAKernelCallExpr *CUDAKernelCallExpr::CreateEmpty(const ASTContext &Ctx,
                                                     unsigned NumArgs,
                                                     EmptyShell Empty) {
   // Allocate storage for the trailing objects of CallExpr.
   unsigned SizeOfTrailingObjects =
       CallExpr::sizeOfTrailingObjects(/*NumPreArgs=*/END_PREARG, NumArgs);
   void *Mem = Ctx.Allocate(sizeof(CUDAKernelCallExpr) + SizeOfTrailingObjects,
                            alignof(CUDAKernelCallExpr));
   return new (Mem) CUDAKernelCallExpr(NumArgs, Empty);
-}
+}
 

clang/lib/Analysis/BodyFarm.cpp

 
 private:
   ASTContext &C;
 };
 }
 
 BinaryOperator *ASTMaker::makeAssignment(const Expr *LHS, const Expr *RHS,
                                          QualType Ty) {
- return new (C) BinaryOperator(const_cast<Expr*>(LHS), const_cast<Expr*>(RHS),
-                               BO_Assign, Ty, VK_RValue,
-                               OK_Ordinary, SourceLocation(), FPOptions());
+  return BinaryOperator::Create(
+      C, const_cast<Expr *>(LHS), const_cast<Expr *>(RHS), BO_Assign, Ty,
+      VK_RValue, OK_Ordinary, SourceLocation(), FPOptions(C.getLangOpts()));
 }
 
 BinaryOperator *ASTMaker::makeComparison(const Expr *LHS, const Expr *RHS,
                                          BinaryOperator::Opcode Op) {
   assert(BinaryOperator::isLogicalOp(Op) ||
          BinaryOperator::isComparisonOp(Op));
-  return new (C) BinaryOperator(const_cast<Expr*>(LHS),
-                                const_cast<Expr*>(RHS),
-                                Op,
-                                C.getLogicalOperationType(),
-                                VK_RValue,
-                                OK_Ordinary, SourceLocation(), FPOptions());
+  return BinaryOperator::Create(C, const_cast<Expr *>(LHS),
+                                const_cast<Expr *>(RHS), Op,
+                                C.getLogicalOperationType(), VK_RValue,
+                                OK_Ordinary, SourceLocation(),
+                                FPOptions(C.getLangOpts()));
 }
 
 CompoundStmt *ASTMaker::makeCompound(ArrayRef<Stmt *> Stmts) {
   return CompoundStmt::Create(C, Stmts, SourceLocation(), SourceLocation());
 }
 
 DeclRefExpr *ASTMaker::makeDeclRefExpr(
     const VarDecl *D,
DeclRefExpr *callOperatorDeclRef =
                           /* T =*/ callOperatorDecl->getType(),
                           /* VK =*/ VK_LValue);
 
   return CXXOperatorCallExpr::Create(
       /*AstContext=*/C, OO_Call, callOperatorDeclRef,
       /*Args=*/CallArgs,
       /*QualType=*/C.VoidTy,
       /*ExprValueType=*/VK_RValue,
-      /*SourceLocation=*/SourceLocation(), FPOptions());
+      /*SourceLocation=*/SourceLocation(),
+      /*FPFeatures=*/FPOptions(C.getLangOpts()));
 }
 
 /// Create a fake body for std::call_once.
 /// Emulates the following function body:
 ///
 /// \code
 /// typedef struct once_flag_s {
 ///   unsigned long __state = 0;

clang/lib/Basic/LangOptions.cpp

 //===- LangOptions.cpp - C Language Family Language Options ---------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
 //  This file defines the LangOptions class.
 //
 //===----------------------------------------------------------------------===//
 
 #include "clang/Basic/LangOptions.h"
+#include "clang/AST/ASTContext.h"
 
 using namespace clang;
 
 LangOptions::LangOptions() {
 #define LANGOPT(Name, Bits, Default, Description) Name = Default;
 #define ENUM_LANGOPT(Name, Type, Bits, Default, Description) set##Name(Default);
 #include "clang/Basic/LangOptions.def"
 }
if (FuncName.equals(NoBuiltinFuncs[i]))
       return true;
   return false;
 }
 
 VersionTuple LangOptions::getOpenCLVersionTuple() const {
   const int Ver = OpenCLCPlusPlus ? OpenCLCPlusPlusVersion : OpenCLVersion;
   return VersionTuple(Ver / 100, (Ver % 100) / 10);
 }
+
+FPOptions FPOptions::defaultWithoutTrailingStorage(const ASTContext &C) {
+  FPOptions result(C.getLangOpts());
+  return result;
+}
+
+bool FPOptions::requiresTrailingStorage(const ASTContext &C) {
+  return getAsOpaqueInt() != defaultWithoutTrailingStorage(C).getAsOpaqueInt();
+}

clang/lib/CodeGen/CGExprScalar.cpp

 
 BinOpInfo ScalarExprEmitter::EmitBinOps(const BinaryOperator *E) {
   TestAndClearIgnoreResultAssign();
   BinOpInfo Result;
   Result.LHS = Visit(E->getLHS());
   Result.RHS = Visit(E->getRHS());
   Result.Ty  = E->getType();
   Result.Opcode = E->getOpcode();
-  Result.FPFeatures = E->getFPFeatures();
+  Result.FPFeatures = E->getFPFeatures(CGF.getContext());
   Result.E = E;
   return Result;
 }
 
 LValue ScalarExprEmitter::EmitCompoundAssignLValue(
                                               const CompoundAssignOperator *E,
                         Value *(ScalarExprEmitter::*Func)(const BinOpInfo &),
                                                    Value *&Result) {
   QualType LHSTy = E->getLHS()->getType();
   BinOpInfo OpInfo;
 
   if (E->getComputationResultType()->isAnyComplexType())
     return CGF.EmitScalarCompoundAssignWithComplex(E, Result);
 
   // Emit the RHS first.  __block variables need to have the rhs evaluated
   // first, plus this should improve codegen a little.
   OpInfo.RHS = Visit(E->getRHS());
   OpInfo.Ty = E->getComputationResultType();
   OpInfo.Opcode = E->getOpcode();
-  OpInfo.FPFeatures = E->getFPFeatures();
+  OpInfo.FPFeatures = E->getFPFeatures(CGF.getContext());
   OpInfo.E = E;
   // Load/convert the LHS.
   LValue LHSLV = EmitCheckedLValue(E->getLHS(), CodeGenFunction::TCK_Store);
 
   llvm::PHINode *atomicPHI = nullptr;
   if (const AtomicType *atomicTy = LHSTy->getAs<AtomicType>()) {
     QualType type = atomicTy->getValueType();
     if (!type->isBooleanType() && type->isIntegerType() &&

clang/lib/CodeGen/CGObjC.cpp

CodeGenFunction::generateObjCSetterBody(const ObjCImplementationDecl *classImpl,
   ImplicitCastExpr argCast(ImplicitCastExpr::OnStack,
                            ivarRef.getType(), argCK, &argLoad,
                            VK_RValue);
   Expr *finalArg = &argLoad;
   if (!getContext().hasSameUnqualifiedType(ivarRef.getType(),
                                            argLoad.getType()))
     finalArg = &argCast;
 
-
-  BinaryOperator assign(&ivarRef, finalArg, BO_Assign,
-                        ivarRef.getType(), VK_RValue, OK_Ordinary,
-                        SourceLocation(), FPOptions());
-  EmitStmt(&assign);
+  BinaryOperator *assign = BinaryOperator::Create(
+      getContext(), &ivarRef, finalArg, BO_Assign, ivarRef.getType(), VK_RValue,
+      OK_Ordinary, SourceLocation(),
+      FPOptions(getContext().getLangOpts()));
+  EmitStmt(assign);
 }
 
 /// Generate an Objective-C property setter function.
 ///
 /// The given Decl must be an ObjCImplementationDecl. \@synthesize
 /// is illegal within a category.
 void CodeGenFunction::GenerateObjCSetter(ObjCImplementationDecl *IMP,
                                          const ObjCPropertyImplDecl *PID) {
DeclRefExpr SrcExpr(getContext(), &SrcDecl, false, SrcTy, VK_RValue,
                       SourceLocation());
   UnaryOperator SRC(&SrcExpr, UO_Deref, SrcTy->getPointeeType(),
                     VK_LValue, OK_Ordinary, SourceLocation(), false);
 
   Expr *Args[2] = { &DST, &SRC };
   CallExpr *CalleeExp = cast<CallExpr>(PID->getSetterCXXAssignment());
   CXXOperatorCallExpr *TheCall = CXXOperatorCallExpr::Create(
       C, OO_Equal, CalleeExp->getCallee(), Args, DestTy->getPointeeType(),
-      VK_LValue, SourceLocation(), FPOptions());
+      VK_LValue, SourceLocation(), FPOptions(C.getLangOpts()));
 
   EmitStmt(TheCall);
 
   FinishFunction();
   HelperFn = llvm::ConstantExpr::getBitCast(Fn, VoidPtrTy);
   CGM.setAtomicSetterHelperFnMap(Ty, HelperFn);
   return HelperFn;
 }

clang/lib/CodeGen/CGStmtOpenMP.cpp

 }
 
 void CodeGenFunction::EmitSections(const OMPExecutableDirective &S) {
   const Stmt *CapturedStmt = S.getInnermostCapturedStmt()->getCapturedStmt();
   const auto *CS = dyn_cast<CompoundStmt>(CapturedStmt);
   bool HasLastprivates = false;
   auto &&CodeGen = [&S, CapturedStmt, CS,
                     &HasLastprivates](CodeGenFunction &CGF, PrePostActionTy &) {
-    ASTContext &C = CGF.getContext();
+    const ASTContext &C = CGF.getContext();
     QualType KmpInt32Ty =
         C.getIntTypeForBitwidth(/*DestWidth=*/32, /*Signed=*/1);
     // Emit helper vars inits.
     LValue LB = createSectionLVal(CGF, KmpInt32Ty, ".omp.sections.lb.",
                                   CGF.Builder.getInt32(0));
     llvm::ConstantInt *GlobalUBVal = CS != nullptr
                                          ? CGF.Builder.getInt32(CS->size() - 1)
                                          : CGF.Builder.getInt32(0);
     LValue UB =
         createSectionLVal(CGF, KmpInt32Ty, ".omp.sections.ub.", GlobalUBVal);
     LValue ST = createSectionLVal(CGF, KmpInt32Ty, ".omp.sections.st.",
                                   CGF.Builder.getInt32(1));
     LValue IL = createSectionLVal(CGF, KmpInt32Ty, ".omp.sections.il.",
                                   CGF.Builder.getInt32(0));
     // Loop counter.
     LValue IV = createSectionLVal(CGF, KmpInt32Ty, ".omp.sections.iv.");
     OpaqueValueExpr IVRefExpr(S.getBeginLoc(), KmpInt32Ty, VK_LValue);
     CodeGenFunction::OpaqueValueMapping OpaqueIV(CGF, &IVRefExpr, IV);
     OpaqueValueExpr UBRefExpr(S.getBeginLoc(), KmpInt32Ty, VK_LValue);
     CodeGenFunction::OpaqueValueMapping OpaqueUB(CGF, &UBRefExpr, UB);
     // Generate condition for loop.
-    BinaryOperator Cond(&IVRefExpr, &UBRefExpr, BO_LE, C.BoolTy, VK_RValue,
-                        OK_Ordinary, S.getBeginLoc(), FPOptions());
+    BinaryOperator *Cond = BinaryOperator::Create(
+        C, &IVRefExpr, &UBRefExpr, BO_LE, C.BoolTy, VK_RValue, OK_Ordinary,
+        S.getBeginLoc(), FPOptions(C.getLangOpts()));
     // Increment for loop counter.
     UnaryOperator Inc(&IVRefExpr, UO_PreInc, KmpInt32Ty, VK_RValue, OK_Ordinary,
                       S.getBeginLoc(), true);
     auto &&BodyGen = [CapturedStmt, CS, &S, &IV](CodeGenFunction &CGF) {
       // Iterate through all sections and emit a switch construct:
       // switch (IV) {
       //   case 0:
       //     <SectionStmt[0]>;
auto &&CodeGen = [&S, CapturedStmt, CS,
     // UB = min(UB, GlobalUB);
     llvm::Value *UBVal = CGF.EmitLoadOfScalar(UB, S.getBeginLoc());
     llvm::Value *MinUBGlobalUB = CGF.Builder.CreateSelect(
         CGF.Builder.CreateICmpSLT(UBVal, GlobalUBVal), UBVal, GlobalUBVal);
     CGF.EmitStoreOfScalar(MinUBGlobalUB, UB);
     // IV = LB;
     CGF.EmitStoreOfScalar(CGF.EmitLoadOfScalar(LB, S.getBeginLoc()), IV);
     // while (idx <= UB) { BODY; ++idx; }
-    CGF.EmitOMPInnerLoop(S, /*RequiresCleanup=*/false, &Cond, &Inc, BodyGen,
+    CGF.EmitOMPInnerLoop(S, /*RequiresCleanup=*/false, Cond, &Inc, BodyGen,
                          [](CodeGenFunction &) {});
     // Tell the runtime we are done.
     auto &&CodeGen = [&S](CodeGenFunction &CGF) {
       CGF.CGM.getOpenMPRuntime().emitForStaticFinish(CGF, S.getEndLoc(),
                                                      S.getDirectiveKind());
     };
     CGF.OMPCancelStack.emitExit(CGF, S.getDirectiveKind(), CodeGen);
     CGF.EmitOMPReductionClauseFinal(S, /*ReductionKind=*/OMPD_parallel);

clang/lib/Frontend/Rewrite/RewriteModernObjC.cpp

if (BaseExpr->getType()->isObjCObjectPointerType()) {
                                                     BaseExpr);
       VarDecl *NewVD = VarDecl::Create(*Context, TUDecl, SourceLocation(),
                                        SourceLocation(), &Context->Idents.get(IvarOffsetName),
                                        Context->UnsignedLongTy, nullptr,
                                        SC_Extern);
       DeclRefExpr *DRE = new (Context)
           DeclRefExpr(*Context, NewVD, false, Context->UnsignedLongTy,
                       VK_LValue, SourceLocation());
-      BinaryOperator *addExpr =
-        new (Context) BinaryOperator(castExpr, DRE, BO_Add,
-                                     Context->getPointerType(Context->CharTy),
-                                     VK_RValue, OK_Ordinary, SourceLocation(), FPOptions());
+      BinaryOperator *addExpr = BinaryOperator::Create(
+          *Context, castExpr, DRE, BO_Add,
+          Context->getPointerType(Context->CharTy), VK_RValue, OK_Ordinary,
+          SourceLocation(), FPOptions(Context->getLangOpts()));
       // Don't forget the parens to enforce the proper binding.
       ParenExpr *PE = new (Context) ParenExpr(SourceLocation(),
                                               SourceLocation(),
                                               addExpr);
       QualType IvarT = D->getType();
       if (D->isBitField())
         IvarT = GetGroupRecordTypeForObjCIvarBitfield(D);
 

clang/lib/Frontend/Rewrite/RewriteObjC.cpp

if (MsgSendStretFlavor) {
     // For X86 it is more complicated and some kind of target specific routine
     // is needed to decide what to do.
     unsigned IntSize =
       static_cast<unsigned>(Context->getTypeSize(Context->IntTy));
     IntegerLiteral *limit = IntegerLiteral::Create(*Context,
                                                    llvm::APInt(IntSize, 8),
                                                    Context->IntTy,
                                                    SourceLocation());
-    BinaryOperator *lessThanExpr =
-      new (Context) BinaryOperator(sizeofExpr, limit, BO_LE, Context->IntTy,
-                                   VK_RValue, OK_Ordinary, SourceLocation(),
-                                   FPOptions());
+    BinaryOperator *lessThanExpr = BinaryOperator::Create(
+        *Context, sizeofExpr, limit, BO_LE, Context->IntTy, VK_RValue,
+        OK_Ordinary, SourceLocation(), FPOptions(Context->getLangOpts()));
     // (sizeof(returnType) <= 8 ? objc_msgSend(...) : objc_msgSend_stret(...))
     ConditionalOperator *CondExpr =
       new (Context) ConditionalOperator(lessThanExpr,
                                         SourceLocation(), CE,
                                         SourceLocation(), STCE,
                                         returnType, VK_RValue, OK_Ordinary);
     ReplacingStmt = new (Context) ParenExpr(SourceLocation(), SourceLocation(),
                                             CondExpr);

clang/lib/Sema/SemaDeclCXX.cpp

buildSingleCopyAssignRecursively(S, Loc, ArrayTy->getElementType(),
                                      Copying, Depth + 1);
   // Bail out if copying fails or if we determined that we should use memcpy.
   if (Copy.isInvalid() || !Copy.get())
     return Copy;
 
   // Create the comparison against the array bound.
   llvm::APInt Upper
     = ArrayTy->getSize().zextOrTrunc(S.Context.getTypeSize(SizeType));
-  Expr *Comparison
-    = new (S.Context) BinaryOperator(IterationVarRefRVal.build(S, Loc),
-                     IntegerLiteral::Create(S.Context, Upper, SizeType, Loc),
-                                     BO_NE, S.Context.BoolTy,
-                                     VK_RValue, OK_Ordinary, Loc, FPOptions());
+  Expr *Comparison = BinaryOperator::Create(
+      S.Context, IterationVarRefRVal.build(S, Loc),
+      IntegerLiteral::Create(S.Context, Upper, SizeType, Loc), BO_NE,
+      S.Context.BoolTy, VK_RValue, OK_Ordinary, Loc, S.FPFeatures);
 
   // Create the pre-increment of the iteration variable. We can determine
   // whether the increment will overflow based on the value of the array
   // bound.
   Expr *Increment = new (S.Context)
       UnaryOperator(IterationVarRef.build(S, Loc), UO_PreInc, SizeType,
                     VK_LValue, OK_Ordinary, Loc, Upper.isMaxValue());
 

clang/lib/Sema/SemaExpr.cpp

static ExprResult convertHalfVecBinOp(Sema &S, ExprResult LHS, ExprResult RHS,
   QualType BinOpResTy = RHS.get()->getType();
 
   // If Opc is a comparison, ResultType is a vector of shorts. In that case,
   // change BinOpResTy to a vector of ints.
   if (isVector(ResultTy, Context.ShortTy))
     BinOpResTy = S.GetSignedVectorType(BinOpResTy);
 
   if (IsCompAssign)
-    return new (Context) CompoundAssignOperator(
-        LHS.get(), RHS.get(), Opc, ResultTy, VK, OK, BinOpResTy, BinOpResTy,
-        OpLoc, FPFeatures);
+    return CompoundAssignOperator::Create(Context, LHS.get(), RHS.get(), Opc,
+                                          ResultTy, VK, OK, OpLoc, FPFeatures,
+                                          BinOpResTy, BinOpResTy);
 
   LHS = convertVector(LHS.get(), Context.FloatTy, S);
-  auto *BO = new (Context) BinaryOperator(LHS.get(), RHS.get(), Opc, BinOpResTy,
-                                          VK, OK, OpLoc, FPFeatures);
+  auto *BO = BinaryOperator::Create(Context, LHS.get(), RHS.get(), Opc,
+                                    BinOpResTy, VK, OK, OpLoc, FPFeatures);
   return convertVector(BO, ResultTy->castAs<VectorType>()->getElementType(), S);
 }
 
 static std::pair<ExprResult, ExprResult>
 CorrectDelayedTyposInBinOp(Sema &S, BinaryOperatorKind Opc, Expr *LHSExpr,
                            Expr *RHSExpr) {
   ExprResult LHS = LHSExpr, RHS = RHSExpr;
   if (!S.getLangOpts().CPlusPlus) {
else if (const ObjCIvarRefExpr *OIRE =
            dyn_cast<ObjCIvarRefExpr>(LHS.get()->IgnoreParenCasts()))
     DiagnoseDirectIsaAccess(*this, OIRE, OpLoc, RHS.get());
 
   // Opc is not a compound assignment if CompResultTy is null.
   if (CompResultTy.isNull()) {
     if (ConvertHalfVec)
       return convertHalfVecBinOp(*this, LHS, RHS, Opc, ResultTy, VK, OK, false,
                                  OpLoc, FPFeatures);
-    return new (Context) BinaryOperator(LHS.get(), RHS.get(), Opc, ResultTy, VK,
-                                        OK, OpLoc, FPFeatures);
+    return BinaryOperator::Create(Context, LHS.get(), RHS.get(), Opc, ResultTy,
+                                  VK, OK, OpLoc, FPFeatures);
   }
 
   // Handle compound assignments.
   if (getLangOpts().CPlusPlus && LHS.get()->getObjectKind() !=
       OK_ObjCProperty) {
     VK = VK_LValue;
     OK = LHS.get()->getObjectKind();
   }
 
   if (ConvertHalfVec)
     return convertHalfVecBinOp(*this, LHS, RHS, Opc, ResultTy, VK, OK, true,
                                OpLoc, FPFeatures);
 
-  return new (Context) CompoundAssignOperator(
-      LHS.get(), RHS.get(), Opc, ResultTy, VK, OK, CompLHSTy, CompResultTy,
-      OpLoc, FPFeatures);
+  return CompoundAssignOperator::Create(Context, LHS.get(), RHS.get(), Opc,
+                                        ResultTy, VK, OK, OpLoc, FPFeatures,
+                                        CompLHSTy, CompResultTy);
 }
 
 /// DiagnoseBitwisePrecedence - Emit a warning when bitwise and comparison
 /// operators are mixed in a way that suggests that the programmer forgot that
 /// comparison operators have higher precedence. The most typical example of
 /// such code is "flags & 0x0020 != 0", which is equivalent to "flags & 1".
 static void DiagnoseBitwisePrecedence(Sema &Self, BinaryOperatorKind Opc,
                                       SourceLocation OpLoc, Expr *LHSExpr,

clang/lib/Sema/SemaExprCXX.cpp

ExprResult Sema::ActOnDecltypeExpression(Expr *E) {
   }
   if (BinaryOperator *BO = dyn_cast<BinaryOperator>(E)) {
     if (BO->getOpcode() == BO_Comma) {
       ExprResult RHS = ActOnDecltypeExpression(BO->getRHS());
       if (RHS.isInvalid())
         return ExprError();
       if (RHS.get() == BO->getRHS())
         return E;
-      return new (Context) BinaryOperator(
-          BO->getLHS(), RHS.get(), BO_Comma, BO->getType(), BO->getValueKind(),
-          BO->getObjectKind(), BO->getOperatorLoc(), BO->getFPFeatures());
+      return BinaryOperator::Create(Context, BO->getLHS(), RHS.get(), BO_Comma,
+                                    BO->getType(), BO->getValueKind(),
+                                    BO->getObjectKind(), BO->getOperatorLoc(),
+                                    BO->getFPFeatures(getASTContext()));
     }
   }
 
   CXXBindTemporaryExpr *TopBind = dyn_cast<CXXBindTemporaryExpr>(E);
   CallExpr *TopCall = TopBind ? dyn_cast<CallExpr>(TopBind->getSubExpr())
                               : nullptr;
   if (TopCall)
     E = TopCall;

clang/lib/Sema/SemaOverload.cpp

if (Fns.empty())
                                          VK_RValue, OK_Ordinary, OpLoc, false);
 
     CXXRecordDecl *NamingClass = nullptr; // lookup ignores member operators
     UnresolvedLookupExpr *Fn = UnresolvedLookupExpr::Create(
         Context, NamingClass, NestedNameSpecifierLoc(), OpNameInfo,
         /*ADL*/ true, IsOverloaded(Fns), Fns.begin(), Fns.end());
     return CXXOperatorCallExpr::Create(Context, Op, Fn, ArgsArray,
                                        Context.DependentTy, VK_RValue, OpLoc,
-                                       FPOptions());
+                                       FPFeatures);
   }
 
   // Build an empty overload set.
   OverloadCandidateSet CandidateSet(OpLoc, OverloadCandidateSet::CSK_Operator);
 
   // Add the candidates from the given function set.
   AddNonMemberOperatorCandidates(Fns, ArgsArray, CandidateSet);
 
if (FnDecl) {
 
       // Determine the result type.
       QualType ResultTy = FnDecl->getReturnType();
       ExprValueKind VK = Expr::getValueKindForType(ResultTy);
       ResultTy = ResultTy.getNonLValueExprType(Context);
 
       Args[0] = Input;
       CallExpr *TheCall = CXXOperatorCallExpr::Create(
-          Context, Op, FnExpr.get(), ArgsArray, ResultTy, VK, OpLoc,
-          FPOptions(), Best->IsADLCandidate);
+          Context, Op, FnExpr.get(), ArgsArray, ResultTy, VK, OpLoc, FPFeatures,
+          Best->IsADLCandidate);
 
       if (CheckCallReturnType(FnDecl->getReturnType(), OpLoc, TheCall, FnDecl))
         return ExprError();
 
       if (CheckFunctionCall(FnDecl, TheCall,
                             FnDecl->getType()->castAs<FunctionProtoType>()))
         return ExprError();
       return CheckForImmediateInvocation(MaybeBindToTemporary(TheCall), FnDecl);
ExprResult Sema::CreateOverloadedBinOp(SourceLocation OpLoc,
 
   // If either side is type-dependent, create an appropriate dependent
   // expression.
   if (Args[0]->isTypeDependent() || Args[1]->isTypeDependent()) {
     if (Fns.empty()) {
       // If there are no functions to store, just build a dependent
       // BinaryOperator or CompoundAssignment.
       if (Opc <= BO_Assign || Opc > BO_OrAssign)
-        return new (Context) BinaryOperator(
-            Args[0], Args[1], Opc, Context.DependentTy, VK_RValue, OK_Ordinary,
-            OpLoc, FPFeatures);
-
-      return new (Context) CompoundAssignOperator(
-          Args[0], Args[1], Opc, Context.DependentTy, VK_LValue, OK_Ordinary,
-          Context.DependentTy, Context.DependentTy, OpLoc,
-          FPFeatures);
+        return BinaryOperator::Create(Context, Args[0], Args[1], Opc,
+                                      Context.DependentTy, VK_RValue,
+                                      OK_Ordinary, OpLoc, FPFeatures);
+      return CompoundAssignOperator::Create(
+          Context, Args[0], Args[1], Opc, Context.DependentTy, VK_LValue,
+          OK_Ordinary, OpLoc, FPFeatures, Context.DependentTy,
+          Context.DependentTy);
     }
 
     // FIXME: save results of ADL from here?
     CXXRecordDecl *NamingClass = nullptr; // lookup ignores member operators
     // TODO: provide better source location info in DNLoc component.
     DeclarationName OpName = Context.DeclarationNames.getCXXOperatorName(Op);
     DeclarationNameInfo OpNameInfo(OpName, OpLoc);
     UnresolvedLookupExpr *Fn = UnresolvedLookupExpr::Create(
if (Result.get()) {
         return ExprError();
     } else {
       Result = ThisResult;
     }
   }
 
   // Build a PseudoObjectExpr to model the rewriting of an <=> operator, and to
   // bind the OpaqueValueExprs before they're (repeatedly) used.
-  Expr *SyntacticForm = new (Context)
-      BinaryOperator(OrigLHS, OrigRHS, BO_Cmp, Result.get()->getType(),
-                     Result.get()->getValueKind(),
-                     Result.get()->getObjectKind(), OpLoc, FPFeatures);
+  Expr *SyntacticForm = BinaryOperator::Create(
+      Context, OrigLHS, OrigRHS, BO_Cmp, Result.get()->getType(),
+      Result.get()->getValueKind(), Result.get()->getObjectKind(), OpLoc,
+      FPFeatures);
   Expr *SemanticForm[] = {LHS, RHS, Result.get()};
   return PseudoObjectExpr::Create(Context, SyntacticForm, SemanticForm, 2);
 }
 
 ExprResult
 Sema::CreateOverloadedArraySubscriptExpr(SourceLocation LLoc,
                                          SourceLocation RLoc,
                                          Expr *Base, Expr *Idx) {
UnresolvedLookupExpr *Fn
                                      NestedNameSpecifierLoc(), OpNameInfo,
                                      /*ADL*/ true, /*Overloaded*/ false,
                                      UnresolvedSetIterator(),
                                      UnresolvedSetIterator());
     // Can't add any actual overloads yet
 
     return CXXOperatorCallExpr::Create(Context, OO_Subscript, Fn, Args,
                                        Context.DependentTy, VK_RValue, RLoc,
-                                       FPOptions());
+                                       FPFeatures);
   }
 
   // Handle placeholders on both operands.
   if (checkPlaceholderForOverload(*this, Args[0]))
     return ExprError();
   if (checkPlaceholderForOverload(*this, Args[1]))
     return ExprError();
 
case OR_Success: {
 
         // Determine the result type
         QualType ResultTy = FnDecl->getReturnType();
         ExprValueKind VK = Expr::getValueKindForType(ResultTy);
         ResultTy = ResultTy.getNonLValueExprType(Context);
 
         CXXOperatorCallExpr *TheCall =
             CXXOperatorCallExpr::Create(Context, OO_Subscript, FnExpr.get(),
-                                        Args, ResultTy, VK, RLoc, FPOptions());
-
+                                        Args, ResultTy, VK, RLoc, FPFeatures);
         if (CheckCallReturnType(FnDecl->getReturnType(), LLoc, TheCall, FnDecl))
           return ExprError();
 
         if (CheckFunctionCall(Method, TheCall,
                               Method->getType()->castAs<FunctionProtoType>()))
           return ExprError();
 
         return MaybeBindToTemporary(TheCall);
       } else {
Sema::BuildCallToObjectOfClassType(Scope *S, Expr *Obj,
 
   // Once we've built TheCall, all of the expressions are properly owned.
   QualType ResultTy = Method->getReturnType();
   ExprValueKind VK = Expr::getValueKindForType(ResultTy);
   ResultTy = ResultTy.getNonLValueExprType(Context);
 
   CXXOperatorCallExpr *TheCall =
       CXXOperatorCallExpr::Create(Context, OO_Call, NewFn.get(), MethodArgs,
-                                  ResultTy, VK, RParenLoc, FPOptions());
+                                  ResultTy, VK, RParenLoc, FPFeatures);
 
   if (CheckCallReturnType(Method->getReturnType(), LParenLoc, TheCall, Method))
     return true;
 
   if (CheckFunctionCall(Method, TheCall, Proto))
     return true;
 
   return CheckForImmediateInvocation(MaybeBindToTemporary(TheCall), Method);
ExprResult FnExpr = CreateFunctionRefExpr(*this, Method, Best->FoundDecl,
                                             Base, HadMultipleCandidates, OpLoc);
   if (FnExpr.isInvalid())
     return ExprError();
 
   QualType ResultTy = Method->getReturnType();
   ExprValueKind VK = Expr::getValueKindForType(ResultTy);
   ResultTy = ResultTy.getNonLValueExprType(Context);
   CXXOperatorCallExpr *TheCall = CXXOperatorCallExpr::Create(
-      Context, OO_Arrow, FnExpr.get(), Base, ResultTy, VK, OpLoc, FPOptions());
+      Context, OO_Arrow, FnExpr.get(), Base, ResultTy, VK, OpLoc, FPFeatures);
 
   if (CheckCallReturnType(Method->getReturnType(), OpLoc, TheCall, Method))
     return ExprError();
 
   if (CheckFunctionCall(Method, TheCall,
                         Method->getType()->castAs<FunctionProtoType>()))
     return ExprError();
 

clang/lib/Sema/SemaPseudoObject.cpp

if (RHS->hasPlaceholderType() || isa<InitListExpr>(RHS)) {
     Semantics.pop_back();
   }
 
   Expr *syntactic;
 
   ExprResult result;
   if (opcode == BO_Assign) {
     result = semanticRHS;
-    syntactic = new (S.Context) BinaryOperator(syntacticLHS, capturedRHS,
-                                               opcode, capturedRHS->getType(),
-                                               capturedRHS->getValueKind(),
-                                               OK_Ordinary, opcLoc,
-                                               FPOptions());
+    syntactic = BinaryOperator::Create(
+        S.Context, syntacticLHS, capturedRHS, opcode, capturedRHS->getType(),
+        capturedRHS->getValueKind(), OK_Ordinary, opcLoc, S.FPFeatures);
+
   } else {
     ExprResult opLHS = buildGet();
     if (opLHS.isInvalid()) return ExprError();
 
     // Build an ordinary, non-compound operation.
     BinaryOperatorKind nonCompound =
       BinaryOperator::getOpForCompoundAssignment(opcode);
     result = S.BuildBinOp(Sc, opcLoc, nonCompound, opLHS.get(), semanticRHS);
     if (result.isInvalid()) return ExprError();
 
-    syntactic =
-      new (S.Context) CompoundAssignOperator(syntacticLHS, capturedRHS, opcode,
-                                             result.get()->getType(),
-                                             result.get()->getValueKind(),
-                                             OK_Ordinary,
-                                             opLHS.get()->getType(),
-                                             result.get()->getType(),
-                                             opcLoc, FPOptions());
+    syntactic = CompoundAssignOperator::Create(
+        S.Context, syntacticLHS, capturedRHS, opcode, result.get()->getType(),
+        result.get()->getValueKind(), OK_Ordinary, opcLoc, S.FPFeatures,
+        opLHS.get()->getType(), result.get()->getType());
   }
 
   // The result of the assignment, if not void, is the value set into
   // the l-value.
   result = buildSet(result.get(), opcLoc, captureSetValueAsResult());
   if (result.isInvalid()) return ExprError();
   addSemanticExpr(result.get());
   if (!captureSetValueAsResult() && !result.get()->getType()->isVoidType() &&
ExprResult Sema::checkPseudoObjectIncDec(Scope *Sc, SourceLocation opcLoc,
   }
 }
 
 ExprResult Sema::checkPseudoObjectAssignment(Scope *S, SourceLocation opcLoc,
                                              BinaryOperatorKind opcode,
                                              Expr *LHS, Expr *RHS) {
   // Do nothing if either argument is dependent.
   if (LHS->isTypeDependent() || RHS->isTypeDependent())
-    return new (Context) BinaryOperator(LHS, RHS, opcode, Context.DependentTy,
-                                        VK_RValue, OK_Ordinary, opcLoc,
-                                        FPOptions());
+    return BinaryOperator::Create(Context, LHS, RHS, opcode,
+                                  Context.DependentTy, VK_RValue, OK_Ordinary,
+                                  opcLoc, FPFeatures);
 
   // Filter out non-overload placeholder types in the RHS.
   if (RHS->getType()->isNonOverloadPlaceholderType()) {
     ExprResult result = CheckPlaceholderExpr(RHS);
     if (result.isInvalid()) return ExprError();
     RHS = result.get();
   }
 
if (UnaryOperator *uop = dyn_cast<UnaryOperator>(syntax)) {
     Expr *op = stripOpaqueValuesFromPseudoObjectRef(*this, uop->getSubExpr());
     return new (Context) UnaryOperator(
         op, uop->getOpcode(), uop->getType(), uop->getValueKind(),
         uop->getObjectKind(), uop->getOperatorLoc(), uop->canOverflow());
   } else if (CompoundAssignOperator *cop
                = dyn_cast<CompoundAssignOperator>(syntax)) {
     Expr *lhs = stripOpaqueValuesFromPseudoObjectRef(*this, cop->getLHS());
     Expr *rhs = cast<OpaqueValueExpr>(cop->getRHS())->getSourceExpr();
-    return new (Context) CompoundAssignOperator(lhs, rhs, cop->getOpcode(),
-                                                cop->getType(),
-                                                cop->getValueKind(),
-                                                cop->getObjectKind(),
-                                                cop->getComputationLHSType(),
-                                                cop->getComputationResultType(),
-                                                cop->getOperatorLoc(),
-                                                FPOptions());
+    return CompoundAssignOperator::Create(
+        Context, lhs, rhs, cop->getOpcode(), cop->getType(),
+        cop->getValueKind(), cop->getObjectKind(), cop->getOperatorLoc(),
+        FPFeatures, cop->getComputationLHSType(),
+        cop->getComputationResultType());
+
   } else if (BinaryOperator *bop = dyn_cast<BinaryOperator>(syntax)) {
     Expr *lhs = stripOpaqueValuesFromPseudoObjectRef(*this, bop->getLHS());
     Expr *rhs = cast<OpaqueValueExpr>(bop->getRHS())->getSourceExpr();
-    return new (Context) BinaryOperator(lhs, rhs, bop->getOpcode(),
-                                        bop->getType(), bop->getValueKind(),
-                                        bop->getObjectKind(),
-                                        bop->getOperatorLoc(), FPOptions());
+    return BinaryOperator::Create(Context, lhs, rhs, bop->getOpcode(),
+                                  bop->getObjectKind(), bop->getOperatorLoc(),
+                                  FPFeatures);
+
   } else if (isa<CallExpr>(syntax)) {
     return syntax;
   } else {
     assert(syntax->hasPlaceholderType(BuiltinType::PseudoObject));
     return stripOpaqueValuesFromPseudoObjectRef(*this, syntax);
   }
 }

clang/lib/Sema/TreeTransform.h

TreeTransform<Derived>::TransformBinaryOperator(BinaryOperator *E) {
   if (RHS.isInvalid())
     return ExprError();
 
   if (!getDerived().AlwaysRebuild() &&
       LHS.get() == E->getLHS() &&
       RHS.get() == E->getRHS())
     return E;
 
+  if (E->isCompoundAssignmentOp())
+    // FPFeatures has already been established from trailing storage
+    return getDerived().RebuildBinaryOperator(
+        E->getOperatorLoc(), E->getOpcode(), LHS.get(), RHS.get());
   Sema::FPFeaturesStateRAII FPFeaturesState(getSema());
-  getSema().FPFeatures = E->getFPFeatures();
+  getSema().FPFeatures = E->getFPFeatures(getSema().getASTContext());
 
   return getDerived().RebuildBinaryOperator(E->getOperatorLoc(), E->getOpcode(),
                                             LHS.get(), RHS.get());
 }
 
 template <typename Derived>
 ExprResult TreeTransform<Derived>::TransformCXXRewrittenBinaryOperator(
     CXXRewrittenBinaryOperator *E) {
ExprResult TreeTransform<Derived>::TransformCXXRewrittenBinaryOperator(
   return getDerived().RebuildCXXRewrittenBinaryOperator(
       E->getOperatorLoc(), Decomp.Opcode, UnqualLookups, LHS.get(), RHS.get());
 }
 
 template<typename Derived>
 ExprResult
 TreeTransform<Derived>::TransformCompoundAssignOperator(
                                                       CompoundAssignOperator *E) {
+  Sema::FPFeaturesStateRAII FPFeaturesState(getSema());
+  getSema().FPFeatures = E->getFPFeatures(getSema().getASTContext());
   return getDerived().TransformBinaryOperator(E);
 }
 
 template<typename Derived>
 ExprResult TreeTransform<Derived>::
 TransformBinaryConditionalOperator(BinaryConditionalOperator *e) {
   // Just rebuild the common and RHS expressions and see whether we
   // get any changes.

clang/lib/Serialization/ASTReaderStmt.cpp

void ASTStmtReader::VisitCastExpr(CastExpr *E) {
   while (NumBaseSpecs--) {
     auto *BaseSpec = new (Record.getContext()) CXXBaseSpecifier;
     *BaseSpec = Record.readCXXBaseSpecifier();
     *BaseI++ = BaseSpec;
   }
 }
 
 void ASTStmtReader::VisitBinaryOperator(BinaryOperator *E) {
+  bool hasFP_Features;
+  BinaryOperator::Opcode opc;
   VisitExpr(E);
+  E->setHasStoredFPFeatures(hasFP_Features = Record.readInt());
+  E->setOpcode(opc = (BinaryOperator::Opcode)Record.readInt());
   E->setLHS(Record.readSubExpr());
   E->setRHS(Record.readSubExpr());
-  E->setOpcode((BinaryOperator::Opcode)Record.readInt());
   E->setOperatorLoc(readSourceLocation());
-  E->setFPFeatures(FPOptions(Record.readInt()));
+  if (hasFP_Features)
+    E->setStoredFPFeatures(FPOptions(Record.readInt()));
 }
 
 void ASTStmtReader::VisitCompoundAssignOperator(CompoundAssignOperator *E) {
   VisitBinaryOperator(E);
   E->setComputationLHSType(Record.readType());
   E->setComputationResultType(Record.readType());
 }
 
while (true) {
     case EXPR_MEMBER:
       S = MemberExpr::CreateEmpty(Context, Record[ASTStmtReader::NumExprFields],
                                   Record[ASTStmtReader::NumExprFields + 1],
                                   Record[ASTStmtReader::NumExprFields + 2],
                                   Record[ASTStmtReader::NumExprFields + 3]);
       break;
 
     case EXPR_BINARY_OPERATOR:
-      S = new (Context) BinaryOperator(Empty);
+      S = BinaryOperator::CreateEmpty(Context,
+                                      Record[ASTStmtReader::NumExprFields]);
       break;
 
     case EXPR_COMPOUND_ASSIGN_OPERATOR:
-      S = new (Context) CompoundAssignOperator(Empty);
+      S = CompoundAssignOperator::CreateEmpty(
+          Context, Record[ASTStmtReader::NumExprFields]);
       break;
 
     case EXPR_CONDITIONAL_OPERATOR:
       S = new (Context) ConditionalOperator(Empty);
       break;
 
     case EXPR_BINARY_CONDITIONAL_OPERATOR:
       S = new (Context) BinaryConditionalOperator(Empty);

clang/lib/Serialization/ASTWriter.cpp

void ASTWriter::WriteDeclContextVisibleUpdate(const DeclContext *DC) {
 
   // Write the lookup table
   RecordData::value_type Record[] = {UPDATE_VISIBLE, getDeclID(cast<Decl>(DC))};
   Stream.EmitRecordWithBlob(UpdateVisibleAbbrev, Record, LookupTable);
 }
 
 /// Write an FP_PRAGMA_OPTIONS block for the given FPOptions.
 void ASTWriter::WriteFPPragmaOptions(const FPOptions &Opts) {
-  RecordData::value_type Record[] = {Opts.getInt()};
+  RecordData::value_type Record[] = {Opts.getAsOpaqueInt()};
   Stream.EmitRecord(FP_PRAGMA_OPTIONS, Record);
 }
 
 /// Write an OPENCL_EXTENSIONS block for the given OpenCLOptions.
 void ASTWriter::WriteOpenCLExtensions(Sema &SemaRef) {
   if (!SemaRef.Context.getLangOpts().OpenCL)
     return;
 

clang/lib/Serialization/ASTWriterStmt.cpp

void ASTStmtWriter::VisitCastExpr(CastExpr *E) {
 
   for (CastExpr::path_iterator
          PI = E->path_begin(), PE = E->path_end(); PI != PE; ++PI)
     Record.AddCXXBaseSpecifier(**PI);
 }
 
 void ASTStmtWriter::VisitBinaryOperator(BinaryOperator *E) {
   VisitExpr(E);
+  bool HasFPFeatures = E->hasStoredFPFeatures();
+  // Write this first for easy access when deserializing, as they affect the
+  // size of the UnaryOperator.
+  Record.push_back(HasFPFeatures);
+  Record.push_back(E->getOpcode()); // FIXME: stable encoding
   Record.AddStmt(E->getLHS());
   Record.AddStmt(E->getRHS());
-  Record.push_back(E->getOpcode()); // FIXME: stable encoding
   Record.AddSourceLocation(E->getOperatorLoc());
-  Record.push_back(E->getFPFeatures().getInt());
+  if (HasFPFeatures)
+    Record.push_back(E->getStoredFPFeatures().getAsOpaqueInt());
   Code = serialization::EXPR_BINARY_OPERATOR;
 }
 
 void ASTStmtWriter::VisitCompoundAssignOperator(CompoundAssignOperator *E) {
   VisitBinaryOperator(E);
   Record.AddTypeRef(E->getComputationLHSType());
   Record.AddTypeRef(E->getComputationResultType());
   Code = serialization::EXPR_COMPOUND_ASSIGN_OPERATOR;
void ASTStmtWriter::VisitMSDependentExistsStmt(MSDependentExistsStmt *S) {
   Record.AddDeclarationNameInfo(S->getNameInfo());
   Record.AddStmt(S->getSubStmt());
   Code = serialization::STMT_MS_DEPENDENT_EXISTS;
 }
 
 void ASTStmtWriter::VisitCXXOperatorCallExpr(CXXOperatorCallExpr *E) {
   VisitCallExpr(E);
   Record.push_back(E->getOperator());
-  Record.push_back(E->getFPFeatures().getInt());
+  Record.push_back(E->getFPFeatures().getAsOpaqueInt());
   Record.AddSourceRange(E->Range);
   Code = serialization::EXPR_CXX_OPERATOR_CALL;
 }
 
 void ASTStmtWriter::VisitCXXMemberCallExpr(CXXMemberCallExpr *E) {
   VisitCallExpr(E);
   Code = serialization::EXPR_CXX_MEMBER_CALL;
 }