[Diffusion] rG76c22b18eafd: [FPEnv][AMDGPU] Correct strictfp tests.

Kevin P. Neal via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Jul 27 06:41:43 PDT 2023


kpn added a comment.

In rG76c22b18eafd2156568d72e9df2ff7bd3457888a#1232438 <https://reviews.llvm.org/rG76c22b18eafd2156568d72e9df2ff7bd3457888a#1232438>, @arsenm wrote:

> In rG76c22b18eafd2156568d72e9df2ff7bd3457888a#1232419 <https://reviews.llvm.org/rG76c22b18eafd2156568d72e9df2ff7bd3457888a#1232419>, @kpn wrote:
>
>> The rules for the strictfp attribute were discussed some time back, and I'm having trouble finding the emails, but some of it was in D69798 <https://reviews.llvm.org/D69798>. There's probably some discussion in the tickets for adding the rules to the Language Reference as well.
>>
>> The strictfp attribute is required by the Inliner. The Inliner needs to know if it is safe to inline a function into a function given that they may have differ on strictfp. Inlining a strictfp function into a non-strictfp function isn't safe without converting the non-strictfp function into a strictfp function, for example.
>>
>> Putting the attribute on just the function definition doesn't work because basic blocks are not required to belong to a function at an arbitrary point during optimization. I believe we ran into a crash when this was tried.
>
> Right but none of this has anything to do with fabs (or more broadly, intrinsics). The verifier doesn't reject fneg in strictfp functions for the same reason. fneg, copysign, fabs, and is.fpclass (and any other theoretical non-canonicalizing op) should not require strictfp call sites

But fneg is an instruction. The current effort is to get tests with call or invoke instructions to follow the rules. And the list of instructions that will be addressed in a follow on set of patches is quite short with fneg on the list.

What about intrinsics that get transformed into calls to real functions? Those will need the strictfp attribute so it can be attached to the call to said real function. Do we have a list of which intrinsics we need to worry about? Will that list be correct, and will it continue to be correct forever for all targets? It seems better to just attach the strictfp attribute to all call and invoke instructions and be certain we have everything covered.


BRANCHES
  main

Users:
  kpn (Author)
  arsenm (Auditor)

https://reviews.llvm.org/rG76c22b18eafd



More information about the llvm-commits mailing list