[PATCH] D43142: Experimental pass to convert all floating point operations to the equivalent constrained intrinsics

Kevin P. Neal via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Nov 2 09:34:37 PDT 2018


kpn added a comment.

In https://reviews.llvm.org/D43142#1284977, @andrew.w.kaylor wrote:

> In https://reviews.llvm.org/D43142#1284800, @hfinkel wrote:
>
> > In https://reviews.llvm.org/D43142#1284271, @cameron.mcinally wrote:
> >
> > > In https://reviews.llvm.org/D43142#1284190, @kpn wrote:
> > >
> > > >
> > >
> >
> >
> > ...
> >
> > > It's unclear to me how LTO (or other cross file inlining) would work here. I haven't given it much though until now. My knee-jerk reaction is that we shouldn't be inlining from a FENV_ACCESS=OFF to FENV_ACCESS=ON location.
> >
> > I'm pretty sure that we decided that we couldn't (because once you inline the regular FP ops, there's no way to restrict their movement relative to the constrained intrinsics at the IR level).
>
>
> I thought what we said was that if constrained FP intrinsics were used anywhere in a function then they had to be used everywhere in the function. So if you inline a function with FENV_ACCESS=ON into a function with FENV_ACCESS=OFF then the FP operations in the destination function need to be converted to constrained intrinsics, and if you inline a function with FENV_ACCESS=OFF into a function with FENV_ACCESS=ON then the FP operations in the function being inlined need to be converted. This conversion will have a detrimental effect on optimizations, so it would be nice to factor that into the inling cost, but at this point I'm mostly concerned about generating correct code.


This. My suggestion to add this pass came from this context right here as described by Andy.

The inlining case inserting regular FP ops into a function is no different from a case where FENV_ACCESS is ON in one block inside a function but OFF in the next block inside that same function. I don't think disabling inlining makes sense because of that.


https://reviews.llvm.org/D43142





More information about the llvm-commits mailing list