On Oct 1, 2012, at 3:02 PM, Lang Hames <[email protected]> wrote: > Hi Doug, > > I've attached a new patch with your suggestions included: the bool field has > been replaced with unsigned, tests have been added, and the default value for > fpContractable has been removed from the constructors. With regards to the > last point: it does force people to give a value for a field that's not > applicable to a lot of cases, which could be confusing. I don't have a strong > objection to it though, so if you think it's the better way to go I'm happy > with that.
I'd rather have people be confused and look it up, rather than omitting it when they should have computed it. > Let me know if this looks ok and I'll commit it. LGTM. > Cheers, > Lang. > > On Sun, Sep 30, 2012 at 9:26 PM, Douglas Gregor <[email protected]> wrote: > > On Sep 26, 2012, at 10:22 PM, Lang Hames <[email protected]> wrote: > >> Hi Hal, >> >> Sorry - that was an oversight on my part. I've now added support for >> expressions of the form (a * b - c) and (c - a * b). Adding that feature >> also prompted me to refactor some of the codegen logic, and testing helped >> me find a couple of bugs where I'd failed to propagate the contractable bit >> during template instantiation. >> >> Updated patch attached. > > Index: include/clang/AST/Expr.h > =================================================================== > --- include/clang/AST/Expr.h (revision 164758) > +++ include/clang/AST/Expr.h (working copy) > @@ -2782,6 +2782,7 @@ > > private: > unsigned Opc : 6; > + bool FPContractable : 1; > SourceLocation OpLoc; > > enum { LHS, RHS, END_EXPR }; > > Please use 'unsigned' rather than 'bool' (due to MSVC packing rules). > > @@ -2790,7 +2791,7 @@ > > BinaryOperator(Expr *lhs, Expr *rhs, Opcode opc, QualType ResTy, > ExprValueKind VK, ExprObjectKind OK, > - SourceLocation opLoc) > + SourceLocation opLoc, bool fpContractable = false) > : Expr(BinaryOperatorClass, ResTy, VK, OK, > lhs->isTypeDependent() || rhs->isTypeDependent(), > lhs->isValueDependent() || rhs->isValueDependent(), > > I suggest not defaulting fpContractable, so that we always have to think > about it when building a new BinaryOperator. > > Are there tests for the template instantiation cases? > > - Doug > > >> Cheers, >> Lang. >> >> On Wed, Sep 26, 2012 at 7:50 PM, Hal Finkel <[email protected]> wrote: >> Lang, >> >> Great, thanks! >> >> Can you please add support for making a*c - b into fma(a, c, -b), etc. >> This is very important for performance on PPC (because this gets >> pattern-matched into a single instruction). >> >> -Hal >> >> On Wed, 26 Sep 2012 17:12:50 -0700 >> Lang Hames <[email protected]> wrote: >> >> > Hi All, >> > >> > This patch adds support for the FP_CONTRACT pragma to clang. It adds >> > a bit to BinaryOperator and CXXOperatorCallExpr to track the >> > FP_CONTRACT pragma state as each AST node is constructed. Pragma >> > state is made to follow scope correctly by having an RAII object save >> > and restore the state when the parser encounters a new compound >> > statement body. The -ffp-contract option is tested during codegen, >> > and fmuladd intrinsics (representing fusing opportunities) are output >> > only if --ffp-contract=on. >> > >> > This patch does NOT include warnings/errors for specifying >> > FP_CONTRACT in invalid contexts that could be confusing (e.g. >> > introducing FP_CONTRACT at class scope). I think it's reasonable to >> > start by supporting valid use cases, and add restrictions/diagnostics >> > in a follow-up patch. >> > >> > Comments and feedback most welcome. >> > >> > Cheers, >> > Lang. >> >> >> >> -- >> Hal Finkel >> Postdoctoral Appointee >> Leadership Computing Facility >> Argonne National Laboratory >> >> <clang-fp-contract-3.patch>_______________________________________________ >> cfe-commits mailing list >> [email protected] >> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits > > > <clang-fp-contract-4.patch>
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
