andrew.w.kaylor added a comment.

Thanks for the patch! I don't have time to review this in detail this week, but 
I'm very happy to see this functionality.



================
Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:890
+  "cannot apply to inline functions, ignoring pragma">,
+   InGroup<IgnoredPragmas>;
 
----------------
rjmccall wrote:
> What's the purpose of this restriction?  Whether `inline` really has much to 
> do with inlining depends a lot on the exact language settings.  (Also, even 
> if this restriction were okay, the diagnostic is quite bad given that there 
> are three separate conditions that can lead to it firing.)
> 
> Also, I thought we were adding instruction-level annotations for this kind of 
> thing to LLVM IR.  Was that not in service of implementing this pragma?
> 
> I'm not categorically opposed to taking patches that only partially implement 
> a feature, but I do want to feel confident that there's a reasonable 
> technical path forward to the full implementation.  In this case, it feels 
> like the function-level attribute is a dead end technically.
I'm guessing this is intended to avoid the optimization problems that would 
occur (currently) if a function with strictfp were inlined into a function 
without it. I'm just guessing though, so correct me if I'm wrong.

As I've said elsewhere, I hope this is a temporary problem. It is a real 
problem though (as is the fact that the inliner isn't currently handling this 
case correctly).

What would you think of a new command line option that caused us to mark 
functions with strictfp as noinline? We'd still need an error somewhat like 
this, but I feel like that would be more likely to accomplish what we want on a 
broad scale.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69272



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to