mibintc marked 9 inline comments as done.
mibintc added a comment.
I added inline comments describing what I did in this version of the patch to
address the bug https://bugs.llvm.org/show_bug.cgi?id=44048
================
Comment at: clang/include/clang/AST/DeclBase.h:1539
+ /// Indicates if the function uses Floating Point Constrained Intrinsics
+ uint64_t UsesFPIntrin : 1;
};
----------------
This corresponds to "strictfp" LLVM attribute. I add this here because I want
to collect the information during Sema and set the attribute during CodeGen.
The next thing I want to do is to add support for modifying float_control via
pragma within function bodies (enable floating point control at the block
level). If I wasn't preparing to support floating_control via statement-level
pragma then setting the bit could be accomplished entirely within CodeGen.
================
Comment at: clang/include/clang/AST/DeclBase.h:1560
/// NumFunctionDeclBitfields or CXXConstructorDeclBitfields.
- uint64_t NumCtorInitializers : 23;
+ uint64_t NumCtorInitializers : 22;
uint64_t IsInheritingConstructor : 1;
----------------
Need to adjust the number of bits here, because it's at the threshold of
overrunning 64 bits.
================
Comment at: clang/include/clang/Basic/DiagnosticDriverKinds.td:444
+def warn_drv_experimental_fp_control_incomplete_opt : Warning<
+ "Support for floating point control option %0 is incomplete and
experimental">,
----------------
@kpn thought it would be a good idea to add a Warning that the implementation
of float control is experimental and partially implemented. That's what this
is for.
================
Comment at: clang/lib/Driver/ToolChains/Clang.cpp:2345
+ << "-ffp-contract=fast";
+ optID = options::OPT_ffp_contract;
+ FPModel = Val;
----------------
michele.scandale wrote:
> Here the state of the floating point options seems unchanged except for
> `FPContract`. If I run `clang -ffp-model=fast -ffp-model=precise`, I would
> expect the state of the floating point options to match the one of
> `-fno-fast-math` except for `FPContract` which you want to be set to "fast".
>
> I think you might need to replicate the reset for all the option here as
> well, so at this point I don't know how much worth is to use the optID reset
> trick for the "fast" case only.
@michele.scandale Thanks for your helpful review, I think I fixed the things
that you remarked on. I also added a test case for the assertion fail that you
saw.
================
Comment at: clang/test/CodeGen/fpconstrained.cpp:10
+
+ template <class>
+ class aaaa {
----------------
This is the test case from the bug report (null deref/segfault/in IRBuilder)
================
Comment at: llvm/include/llvm/IR/IRBuilder.h:268
I->addAttribute(AttributeList::FunctionIndex, Attribute::StrictFP);
- setConstrainedFPFunctionAttr();
}
----------------
@kpn I got rid of this line because the function attribute is being set in
CodeGen
================
Comment at: llvm/unittests/IR/IRBuilderTest.cpp:186
Builder.setIsFPConstrained(true);
+ auto Parent = BB->getParent();
+ Parent->addFnAttr(Attribute::StrictFP);
----------------
@kpn I changed the test to create the function attribute a priori since it will
be set in CodeGen before passing to IRBuilder
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D62731/new/
https://reviews.llvm.org/D62731
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
