[cfe-commits] [PATCH] FP_CONTRACT pragma support.

[Lang Hames]

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.

Let me know if this looks ok and I'll commit it.

Cheers,
Lang.

On Sun, Sep 30, 2012 at 9:26 PM, Douglas Gregor <dgregor at apple.com> wrote:

>
> On Sep 26, 2012, at 10:22 PM, Lang Hames <lhames at gmail.com> 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 <hfinkel at anl.gov> 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 <lhames at gmail.com> 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
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20121001/66ae4220/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: clang-fp-contract-4.patch
Type: application/octet-stream
Size: 30290 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20121001/66ae4220/attachment.obj>