<div dir="ltr"><div>I don't see any controversy for the preliminary requirement of removing <span class="gmail-im">BinaryOperator::isFNeg() and friends, so start with that?</span></div><div><span class="gmail-im">That work may reveal other potential regressions that we can patch in advance too.</span></div><div><span class="gmail-im"><br></span></div><div><span class="gmail-im">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). <br></span></div><div><span class="gmail-im"><br></span></div><div><span class="gmail-im">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?<br></span></div><div><span class="gmail-im"></span></div></div><br><div class="gmail_quote"><div dir="ltr">On Mon, Oct 1, 2018 at 12:20 PM Cameron McInally <<a href="mailto:cameron.mcinally@nyu.edu">cameron.mcinally@nyu.edu</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">On Thu, Sep 27, 2018 at 10:14 AM Sanjay Patel <<a href="mailto:spatel@rotateright.com" target="_blank">spatel@rotateright.com</a>> wrote:<br><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div dir="ltr"><div dir="ltr">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. :)<div>See the discussion and related links here: <a href="https://reviews.llvm.org/D19391" target="_blank">https://reviews.llvm.org/D19391</a><br></div><div><br></div><div>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).<br></div><div><br></div><div>So that leads back to the m_FNeg abstraction - it will have to match both ops to not lose optimizations.</div></div></div></div></blockquote><div><br></div><div>I like this idea. </div><div><br></div><div>So how do we get official approval to begin this work?  </div></div></div>
</blockquote></div>