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

[Lang Hames]

Thanks Doug,

Patch applied with a few additional comments in r164989.

Cheers,
Lang.

On Mon, Oct 1, 2012 at 3:07 PM, Douglas Gregor <dgregor at apple.com> wrote:

>
> On Oct 1, 2012, at 3:02 PM, Lang Hames <lhames at gmail.com> 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 <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
>>
>>
>>
> <clang-fp-contract-4.patch>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20121001/66a23e0a/attachment.html>