[PATCH] D92596: [FPEnv] Correct constrained metadata in fp16-ops-strict.c

Mon Dec 7 08:12:55 PST 2020

kpn added inline comments.

================
Comment at: clang/lib/CodeGen/CGExprScalar.cpp:3006

+  CodeGenFunction::CGFPOptionsRAII FPOptsRAII(CGF, OpInfo.FPFeatures);
   SourceLocation Loc = E->getExprLoc();
----------------
mibintc wrote:
> kpn wrote:
> > mibintc wrote:
> > > What's the rule to follow about when we need to FPOptsRAII? 
> > It is used on the border between code that has the AST node and code that doesn't. If any code below this point might use the constrained floating point intrinsics then the FPOptsRAII is needed.
> > 
> > Sometimes this border is at a call to the IRBuilder. Sometimes it's buried elsewhere. The hope is that by having the location be defined there we can at some point audit to verify we have it everywhere we should.
> Oh, that doesn't sound very bug-proof.  Do you mind pointing out 2 different instances, one "AST node" and one "buried elsewhere"?  Or there's probably a code review I should read to find this?  Thank you
Agreed, but the approach of wrapping calls to the IRBuilder isn't very bug-proof either. And it requires threading strictfp info throughout code that otherwise doesn't support carrying around info currently. So it would be invasive and hard to audit.

This patch is an example of code that has the AST node (the Expr*) and is calling the IRBuilder. For "buried" examples see D88913.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92596