On Wed, Sep 26, 2018 at 9:32 AM Sanjay Patel <spatel at rotateright.com> wrote:> > > On Tue, Sep 25, 2018 at 7:47 PM Cameron McInally <cameron.mcinally at nyu.edu> > wrote: > >> >> This is the first time I'm looking at foldShuffledBinop(...), so maybe a >> naive question, but why not do similar shuffle canonicalizations on unary >> (or ternary) operations? That may be a better fix in the long run. >> > > AFAIK, all of the math/logic folding that we do currently is on binary > operators because all of the instructions have that form: > http://llvm.org/docs/LangRef.html#instruction-reference > > As discussed, we fake the unary neg/not/fneg as binops. Excluding > control-flow, the only unary instructions are casts, and I don't see any > ternary or higher math ops other than intrinsics. >Digressing a bit... That sounds like a bug, not a feature. Casts/converts, rounds, abs, other libm functions, fmas, compares, and probably more can be masked. If intermixed shuffles are preventing combines on those, intrinsics or not, that isn't ideal. -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20180926/6deec4ac/attachment.html>
On Wed, Sep 26, 2018 at 9:42 AM Cameron McInally <cameron.mcinally at nyu.edu> wrote:> On Wed, Sep 26, 2018 at 9:32 AM Sanjay Patel <spatel at rotateright.com> > wrote: > >> >> >> On Tue, Sep 25, 2018 at 7:47 PM Cameron McInally < >> cameron.mcinally at nyu.edu> wrote: >> >>> >>> This is the first time I'm looking at foldShuffledBinop(...), so maybe a >>> naive question, but why not do similar shuffle canonicalizations on unary >>> (or ternary) operations? That may be a better fix in the long run. >>> >> >> AFAIK, all of the math/logic folding that we do currently is on binary >> operators because all of the instructions have that form: >> http://llvm.org/docs/LangRef.html#instruction-reference >> >> As discussed, we fake the unary neg/not/fneg as binops. Excluding >> control-flow, the only unary instructions are casts, and I don't see any >> ternary or higher math ops other than intrinsics. >> > > Digressing a bit... > > That sounds like a bug, not a feature. Casts/converts, rounds, abs, other > libm functions, fmas, compares, and probably more can be masked. If > intermixed shuffles are preventing combines on those, intrinsics or not, > that isn't ideal. >Casts and compares have plenty of specialized transforms, so I think we have that covered. Similarly, we have libm and intrinsic transforms split between LibCallSimplifier and InstCombiner::visitCallInst(). So we could do better to organize things, but there probably aren't too many holes in those optimizations (or at least I haven't seen them yet). To bring it back to the question of fneg, let me know if this is an accurate summary: 1. fneg would be nice to have for clarity, but it doesn't make optimization in the default LLVM FP environment any easier/better. 2. We will have to do some preliminary work in the IR optimizer to avoid regressions if we add fneg to the IR. 3. We want fneg as a 1st class instruction even though the related fabs/copysign bitstring ops are intrinsics (because fneg is more common than the others?). 4. Adding fneg to IR means we do not need to add a constrained intrinsic for fneg (likewise, there's no need for constrained fabs/copysign because those intrinsics already exist). -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20180926/ddc13885/attachment.html>
Do we really want to have fneg be the only instruction with guaranteed no side effects? That just sounds like a gotcha waiting to happen. Or it could result in horrible code depending on the architecture. I’m still leaning towards having both an intrinsic and an instruction, and if they happen to have the same behavior then that’s fine. If fneg is to be a special instruction with extra promises (vs the other FP instructions) then this fact needs to be loudly present in the documentation. -- Kevin P. Neal SAS/C and SAS/C++ Compiler Host Research and Development SAS Institute, Inc. From: llvm-dev <llvm-dev-bounces at lists.llvm.org> On Behalf Of Sanjay Patel via llvm-dev Sent: Wednesday, September 26, 2018 1:25 PM To: cameron.mcinally at nyu.edu Cc: llvm-dev <llvm-dev at lists.llvm.org> Subject: Re: [llvm-dev] [FPEnv] FNEG instruction To bring it back to the question of fneg, let me know if this is an accurate summary: 3. We want fneg as a 1st class instruction even though the related fabs/copysign bitstring ops are intrinsics (because fneg is more common than the others?). 4. Adding fneg to IR means we do not need to add a constrained intrinsic for fneg (likewise, there's no need for constrained fabs/copysign because those intrinsics already exist). -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20180926/158a5513/attachment.html>
> > On Wed, Sep 26, 2018 at 1:24 PM Sanjay Patel <spatel at rotateright.com> > wrote: > ... > >> To bring it back to the question of fneg, let me know if this is an >> accurate summary: >> 1. fneg would be nice to have for clarity, but it doesn't make >> optimization in the default LLVM FP environment any easier/better. >> 2. We will have to do some preliminary work in the IR optimizer to avoid >> regressions if we add fneg to the IR. >> 3. We want fneg as a 1st class instruction even though the related >> fabs/copysign bitstring ops are intrinsics (because fneg is more common >> than the others?). >> 4. Adding fneg to IR means we do not need to add a constrained intrinsic >> for fneg (likewise, there's no need for constrained fabs/copysign because >> those intrinsics already exist). >> >> >For simplicity's sake, I agree with all this. I could nit-pick a bit, but it's not productive to the FNEG conversation. Thanks for summarizing, Sanjay. -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20180926/6392c1ba/attachment.html>