[PATCH] D158695: [clang] Fix missing contract flag in sqrt intrinsic

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl marked 4 inline comments as done.
yaxunl added inline comments.


================
Comment at: clang/lib/CodeGen/CGBuiltin.cpp:501
   if (CGF.Builder.getIsFPConstrained()) {
     CodeGenFunction::CGFPOptionsRAII FPOptsRAII(CGF, E);
     Function *F = CGF.CGM.getIntrinsic(ConstrainedIntrinsicID, Src0->getType());
----------------
arsenm wrote:
> rjmccall wrote:
> > Is this existing condition not good enough, and why?
> It's only in the strictfp branch for some reason, I don't think both would be needed
It seems I just need to move the RAII out of that condition.


================
Comment at: clang/test/CodeGen/fp-contract-fast-pragma.cpp:2
 // RUN: %clang_cc1 -O3 -triple %itanium_abi_triple -emit-llvm -o - %s | FileCheck %s
 
 // Is FP_CONTRACT honored in a simple case?
----------------
arsenm wrote:
> Should also check constrained run line
will do


================
Comment at: clang/test/CodeGen/fp-contract-fast-pragma.cpp:11
 #pragma clang fp contract(fast)
-  return a * b + c;
+  return a * b + c + __builtin_sqrtf(a);
 }
----------------
arsenm wrote:
> Should leave the existing test function alone and add a new one. Also can you test some cases with nested different values
pragma clang fp cannot be used inside compound statements. It is only allowed in either function scope or at the beginning of a compound statement. Essentially, it can only enable/disable contracting at a granularity of the function level. I can add a test for enabling/disabling it in different functions.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D158695/new/

https://reviews.llvm.org/D158695