Hi Doug,<div><br></div><div>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.</div>
<div><br></div><div>Let me know if this looks ok and I'll commit it.</div><div><br></div><div>Cheers,</div><div>Lang.<br><br><div class="gmail_quote">On Sun, Sep 30, 2012 at 9:26 PM, Douglas Gregor <span dir="ltr"><<a href="mailto:dgregor@apple.com" target="_blank">dgregor@apple.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word"><br><div><div class="im"><div>On Sep 26, 2012, at 10:22 PM, Lang Hames <<a href="mailto:lhames@gmail.com" target="_blank">lhames@gmail.com</a>> wrote:</div>
<br><blockquote type="cite">Hi Hal,<div><br></div><div>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.</div>
<div><br></div><div>Updated patch attached.</div></blockquote><div><br></div></div><div><div>Index: include/clang/AST/Expr.h</div><div>===================================================================</div><div>--- include/clang/AST/Expr.h<span style="white-space:pre-wrap"> </span>(revision 164758)</div>
<div>+++ include/clang/AST/Expr.h<span style="white-space:pre-wrap"> </span>(working copy)</div><div>@@ -2782,6 +2782,7 @@</div><div> </div><div> private:</div><div> unsigned Opc : 6;</div><div>+ bool FPContractable : 1;</div>
<div> SourceLocation OpLoc;</div><div> </div><div> enum { LHS, RHS, END_EXPR };</div><div><br></div><div>Please use 'unsigned' rather than 'bool' (due to MSVC packing rules).</div><div><br></div><div><div>
@@ -2790,7 +2791,7 @@</div><div> </div><div> BinaryOperator(Expr *lhs, Expr *rhs, Opcode opc, QualType ResTy,</div><div> ExprValueKind VK, ExprObjectKind OK,</div><div>- SourceLocation opLoc)</div>
<div>+ SourceLocation opLoc, bool fpContractable = false)</div><div> : Expr(BinaryOperatorClass, ResTy, VK, OK,</div><div> lhs->isTypeDependent() || rhs->isTypeDependent(),</div><div> lhs->isValueDependent() || rhs->isValueDependent(),</div>
</div><div><br></div><div>I suggest not defaulting fpContractable, so that we always have to think about it when building a new BinaryOperator.</div><div><br></div><div>Are there tests for the template instantiation cases?</div>
<div><br></div></div><span style="white-space:pre-wrap"> </span>- Doug</div><div><br></div><div><br><blockquote type="cite"><div><div class="h5"><div>Cheers,</div><div>Lang.<br><br><div class="gmail_quote">On Wed, Sep 26, 2012 at 7:50 PM, Hal Finkel <span dir="ltr"><<a href="mailto:hfinkel@anl.gov" target="_blank">hfinkel@anl.gov</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Lang,<br>
<br>
Great, thanks!<br>
<br>
Can you please add support for making a*c - b into fma(a, c, -b), etc.<br>
This is very important for performance on PPC (because this gets<br>
pattern-matched into a single instruction).<br>
<br>
-Hal<br>
<div><div><br>
On Wed, 26 Sep 2012 17:12:50 -0700<br>
Lang Hames <<a href="mailto:lhames@gmail.com" target="_blank">lhames@gmail.com</a>> wrote:<br>
<br>
> Hi All,<br>
><br>
> This patch adds support for the FP_CONTRACT pragma to clang. It adds<br>
> a bit to BinaryOperator and CXXOperatorCallExpr to track the<br>
> FP_CONTRACT pragma state as each AST node is constructed. Pragma<br>
> state is made to follow scope correctly by having an RAII object save<br>
> and restore the state when the parser encounters a new compound<br>
> statement body. The -ffp-contract option is tested during codegen,<br>
> and fmuladd intrinsics (representing fusing opportunities) are output<br>
> only if --ffp-contract=on.<br>
><br>
> This patch does NOT include warnings/errors for specifying<br>
> FP_CONTRACT in invalid contexts that could be confusing (e.g.<br>
> introducing FP_CONTRACT at class scope). I think it's reasonable to<br>
> start by supporting valid use cases, and add restrictions/diagnostics<br>
> in a follow-up patch.<br>
><br>
> Comments and feedback most welcome.<br>
><br>
> Cheers,<br>
> Lang.<br>
<br>
<br>
<br>
</div></div><span><font color="#888888">--<br>
Hal Finkel<br>
Postdoctoral Appointee<br>
Leadership Computing Facility<br>
Argonne National Laboratory<br>
</font></span></blockquote></div><br></div>
</div></div><span><clang-fp-contract-3.patch></span>_______________________________________________<br>cfe-commits mailing list<br><a href="mailto:cfe-commits@cs.uiuc.edu" target="_blank">cfe-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits</a><br></blockquote></div><br></div></blockquote></div><br></div>