I don't see any controversy for the preliminary requirement of removing BinaryOperator::isFNeg() and friends, so start with that? That work may reveal other potential regressions that we can patch in advance too. Other than that, I think there's really only a question of do we want 1 or both of fneg and fneg_constrained (and if we choose both, then I assume we'd also add fabs_constrained and copysign_constrained). I don't see why we want those redundancies. As discussed here, we already can't canonicalize to the FP bitops in IR because of non-IEEE targets (D19391). In IR and the backend, you don't want to over-constrain allowable optimizations. Fneg folds shouldn't be disabled just because we changed the FP exception state? On Mon, Oct 1, 2018 at 12:20 PM Cameron McInally <cameron.mcinally at nyu.edu> wrote:> On Thu, Sep 27, 2018 at 10:14 AM Sanjay Patel <spatel at rotateright.com> > wrote: > >> Regarding non-IEEE targets: yes, we definitely support those, so we do >> have to be careful about not breaking them. I know because I have broken >> them. :) >> See the discussion and related links here: >> https://reviews.llvm.org/D19391 >> >> But having an exactly specified fneg op makes that easier, not harder, as >> I see it. Unfortunately, if a target doesn't support this op (always toggle >> the sign bit and only the sign bit), then we can't canonicalize 'fsub -0.0, >> X' to 'fneg X' because those are not identical ops (denorms, NaN). >> >> So that leads back to the m_FNeg abstraction - it will have to match both >> ops to not lose optimizations. >> > > I like this idea. > > So how do we get official approval to begin this work? >-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20181001/a5f6e724/attachment.html>
If we don’t have constrained intrinsics for some of the fp math instructions then aren’t we risking non-strict optimizations? -- Kevin P. Neal SAS/C and SAS/C++ Compiler Host Research and Development SAS Institute, Inc. From: Sanjay Patel <spatel at rotateright.com> Sent: Monday, October 01, 2018 5:41 PM To: cameron.mcinally at nyu.edu Cc: Kevin Neal <Kevin.Neal at sas.com>; llvm-dev <llvm-dev at lists.llvm.org> Subject: Re: [llvm-dev] [FPEnv] FNEG instruction EXTERNAL I don't see any controversy for the preliminary requirement of removing BinaryOperator::isFNeg() and friends, so start with that? That work may reveal other potential regressions that we can patch in advance too. Other than that, I think there's really only a question of do we want 1 or both of fneg and fneg_constrained (and if we choose both, then I assume we'd also add fabs_constrained and copysign_constrained). I don't see why we want those redundancies. As discussed here, we already can't canonicalize to the FP bitops in IR because of non-IEEE targets (D19391). In IR and the backend, you don't want to over-constrain allowable optimizations. Fneg folds shouldn't be disabled just because we changed the FP exception state? On Mon, Oct 1, 2018 at 12:20 PM Cameron McInally <cameron.mcinally at nyu.edu<mailto:cameron.mcinally at nyu.edu>> wrote: On Thu, Sep 27, 2018 at 10:14 AM Sanjay Patel <spatel at rotateright.com<mailto:spatel at rotateright.com>> wrote: Regarding non-IEEE targets: yes, we definitely support those, so we do have to be careful about not breaking them. I know because I have broken them. :) See the discussion and related links here: https://reviews.llvm.org/D19391 But having an exactly specified fneg op makes that easier, not harder, as I see it. Unfortunately, if a target doesn't support this op (always toggle the sign bit and only the sign bit), then we can't canonicalize 'fsub -0.0, X' to 'fneg X' because those are not identical ops (denorms, NaN). So that leads back to the m_FNeg abstraction - it will have to match both ops to not lose optimizations. I like this idea. So how do we get official approval to begin this work? -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20181002/234ef760/attachment.html>
On Mon, Oct 1, 2018 at 5:41 PM Sanjay Patel <spatel at rotateright.com> wrote:> I don't see any controversy for the preliminary requirement of removing BinaryOperator::isFNeg() > and friends, so start with that? > That work may reveal other potential regressions that we can patch in > advance too. >This is true and I will agree to do this work...> Other than that, I think there's really only a question of do we want 1 or > both of fneg and fneg_constrained (and if we choose both, then I assume > we'd also add fabs_constrained and copysign_constrained). >but this is the real goal. Doing the BinaryOperator::isFNeg() work is in vain if we don't have at least a conditional approval of an explicit FNEG IR instruction. Would it be possible to obtain that conditional approval before work begins? That seems most prudent. -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20181002/7a2ae7b7/attachment.html>
On Tue, Oct 2, 2018 at 10:06 PM Cameron McInally via llvm-dev <llvm-dev at lists.llvm.org> wrote:> > On Mon, Oct 1, 2018 at 5:41 PM Sanjay Patel <spatel at rotateright.com> wrote: >> >> I don't see any controversy for the preliminary requirement of removing BinaryOperator::isFNeg() and friends, so start with that? >> That work may reveal other potential regressions that we can patch in advance too. > > > This is true and I will agree to do this work... > >> >> Other than that, I think there's really only a question of do we want 1 or both of fneg and fneg_constrained (and if we choose both, then I assume we'd also add fabs_constrained and copysign_constrained). > > > but this is the real goal. Doing the BinaryOperator::isFNeg() work is in vain if we don't have at least a conditional approval of an explicit FNEG IR instruction. > > Would it be possible to obtain that conditional approval before work begins? That seems most prudent.Will this affect (regress, pessimize) the current optimizations for non-strict cases? What about -ffast-math? Roman.> _______________________________________________ > LLVM Developers mailing list > llvm-dev at lists.llvm.org > http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
On Tue, Oct 2, 2018 at 12:09 PM Kevin Neal <Kevin.Neal at sas.com> wrote:> If we don’t have constrained intrinsics for some of the fp math > instructions then aren’t we risking non-strict optimizations? >So far we've only added constrained FP intrinsics for operations that have side effects (i.e. can trap). The quiet-computational sign-bit operations are special. They never have side effects (i.e. never trap). We have not been able to find a bad transformation for those (i.e. a xform to a trapping operation) other than the FNEG(X) --> FSUB(-0.0, X) transform. If you can find a transform from one of the quiet-computational operations to a trapping operation, then we should discuss that. -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20181002/e247607d/attachment.html>
On Tue, Oct 2, 2018 at 1:06 PM Cameron McInally <cameron.mcinally at nyu.edu> wrote:> On Mon, Oct 1, 2018 at 5:41 PM Sanjay Patel <spatel at rotateright.com> > wrote: > >> I don't see any controversy for the preliminary requirement of removing BinaryOperator::isFNeg() >> and friends, so start with that? >> That work may reveal other potential regressions that we can patch in >> advance too. >> > > This is true and I will agree to do this work... > > >> Other than that, I think there's really only a question of do we want 1 >> or both of fneg and fneg_constrained (and if we choose both, then I assume >> we'd also add fabs_constrained and copysign_constrained). >> > > but this is the real goal. Doing the BinaryOperator::isFNeg() work is in > vain if we don't have at least a conditional approval of an explicit FNEG > IR instruction. > > Would it be possible to obtain that conditional approval before work > begins? That seems most prudent. >I can't speak for anyone else, but I approve for the same reasons that were mentioned early on: fneg makes the intended behavior and potential transforms easier to discern. I don't have much knowledge of the problems/requirements in the constrained environment, so I'll leave the constrained counterpart decision to people who are working on that. -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20181002/2d54ea29/attachment.html>
On Tue, Oct 2, 2018 at 10:09 AM Kevin Neal <Kevin.Neal at sas.com> wrote:> If we don’t have constrained intrinsics for some of the fp math > instructions then aren’t we risking non-strict optimizations? >As I said in my last reply, I haven't followed the constrained work, so I don't know...but I'm curious. What is the optimized LLVM IR for this C source assuming the frontend recognizes/respects the pragma: #pragma STDC FENV_ACCESS ON float negneg(float x, float y) { return (-x) * (-y); } -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20181002/6203f51e/attachment.html>