[PATCH] D69272: Restricted variant of '#pragma STDC FENV_ACCESS'

Serge Pavlov via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Oct 22 09:45:44 PDT 2019


sepavloff added a comment.

In D69272#1717967 <https://reviews.llvm.org/D69272#1717967>, @kpn wrote:

> Baking into the front end the fact that the backend implementation is not yet complete doesn't strike me as a good idea.


I don't expect that this patch would pass review quickly. But it could be used to organize requests what must be done to implement this functionality. Now I see that we need to add functionality to inliner. Do you have ideas what else should be done to implement this variant of the pragma?

> I think the issue with the inliner not being smart enough yet is an issue for llvm to deal with and not front ends like clang. It would be straightforward enough for llvm to mark functions that have the strictfp attribute so they also are marked noinline. As a temporary measure, of course. This is a case where llvm hasn't caught up with well-formed IR, so it would be llvm's job to work around its own incompleteness.

That's true. But if user specifies `inline` for a function that contains the pragma, he requested contradictory attributes. Should compiler emit a warning? If yes, this is a job of frontend.

> See D43142 <https://reviews.llvm.org/D43142> for code to convert all floating point in a function into constrained intrinsics. Updated versions of this code with support for intrinsics that didn't exist at the time also exist. I don't see why this pass couldn't be reworked a bit to be used by the inliner. And it would only be needed when inlining into a strictfp function a function that wasn't strictfp.

I think this patch can help in adaptation of the inliner. The transformation must be rewritten as a function or even built into the logic of inliner.

> You mentioned that extending the scope of the #pragma may result in a "performance drop, which is unacceptable in many cases". But the only difference between allowing the #pragma only at the top of a function, and allowing it everywhere the standard allows, is that the user knows about the potential loss of performance. The performance loss happens in both cases. Again, I don't think baking into clang the current state of llvm is a good idea.

A user may be convinced that using the pragma is expensive. He would carefully implement the function with the pragma and if the function is small and called rarely, performance drop can be minimized. Such solution does not work for all cases but for some it is acceptable. If using the pragma in a small region results in loss of optimization in entire function, this is counterintuitive and in many cases unacceptable.

> A warning from clang that strictfp code doesn't perform very well today is probably a good idea, and it would be ripped out easily when the day comes. The warning would only fire when the #pragma is seen, and that code is small, self-contained, and actually already exists in clang now but with different text.

I don't see how such warning can help a user. A note about impact of the pragma on performance can be put into documentation. Issuing a warning on every use of the pragma may be annoying.

The main advantage of this restricted variant of the pragma IMHO is the possibility to provide implementation which can be developed in reasonable time and can be used in production code. Users will file bugs, we will fix them and the implementation will progress. If the pragma causes substantial performance loss, number of its users will be lower and the development will be slowed down.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69272





More information about the cfe-commits mailing list