rjmccall added inline comments.
================
Comment at: clang/lib/Serialization/ASTReader.cpp:7899
+ if (FpPragmaCurrentLocation.isInvalid()) {
+ assert(*FpPragmaCurrentValue == SemaObj->FpPragmaStack.DefaultValue &&
+ "Expected a default pragma float_control value");
----------------
mibintc wrote:
> yaxunl wrote:
> > mibintc wrote:
> > > yaxunl wrote:
> > > > This changes the behavior regarding AST reader and seems to be too hash
> > > > restriction. Essentially this requires a pch can only be used with the
> > > > same fp options with which the pch is generated. Since there are lots
> > > > of fp options, it is impractical to generate pch for all the
> > > > combinations.
> > > >
> > > > We have seen regressions due to this assertion.
> > > >
> > > > Can this assertion be dropped or done under some options?
> > > >
> > > > Thanks.
> > > >
> > > @yaxunl Can you please send me a reproducer, I'd like to see what's going
> > > on, not sure if just getting rid of the assertion will give the desired
> > > outcome.
> > {F11915161}
> >
> > Pls apply the patch.
> >
> > Thanks.
> @rjmccall In the example supplied by @yaxunl, the floating point options in
> the pch file when created are default, and the floating point options in the
> use have no-signed-zeros flag. The discrepancy causes an error diagnostic
> when the pch is used. I added the FMF flags into FPFeatures in this patch, I
> made them COMPATIBLE_LANGOPT which is the encoding also being used for
> FastMath, FiniteMathOnly, and UnsafeFPMath. Do you have some advice about
> this issue?
A couple things are going on here.
First: a PCH can only end at the top level, not in the middle of a declaration,
but otherwise Sema can be in an arbitrary semantic configuration. That
definitely includes arbitrary pragmas being in effect, so in general the end
state might not match the default FP state, so this assertion is bogus. When
loading a PCH, you need to restore the pragma stack and current FP state to the
configuration it was in at the end of the PCH.
Second: if you restore the pragma stack and FP state naively given the current
representation of FP state, you will completely overwrite the FP settings of
the current translation unit with the FP settings that were in effect when the
PCH was built, which is obviously not okay. This is one way (among several)
that the current representation is not really living up to the statement that
these language options are "compatible". The better way to do this would be
for the pragma stack and Expr nodes to record the current set of overrides in
effect rather than the absolute current state; this could then be easily
applied to an arbitrary global FP state.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D72841/new/
https://reviews.llvm.org/D72841
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
