[PATCH] D93586: [InstCombine] use poison as placeholder for undemanded elems

2020-12-27 Thread Juneyoung Lee via Phabricator via cfe-commits
aqjune added a comment. Okay, I'll gently land this. I was just wondering whether insertelements having heterogenous placeholder would be problematic (this patch makes some of insertelement use poison, but not all), but it may not matter very much. Repository: rG LLVM Github Monorepo CHANGE

[PATCH] D93586: [InstCombine] use poison as placeholder for undemanded elems

2020-12-27 Thread Nikita Popov via Phabricator via cfe-commits
nikic added a comment. In D93586#2472314 , @aqjune wrote: > There are 3 more patches: > > https://reviews.llvm.org/D93793 (IRBuilder's CreateVectorSplat) > https://reviews.llvm.org/D93817 (Other transformations) > https://reviews.llvm.org/D93818 (LangRef)

[PATCH] D93586: [InstCombine] use poison as placeholder for undemanded elems

2020-12-27 Thread Juneyoung Lee via Phabricator via cfe-commits
aqjune added a comment. There are 3 more patches: https://reviews.llvm.org/D93793 (IRBuilder's CreateVectorSplat) https://reviews.llvm.org/D93817 (Other transformations) https://reviews.llvm.org/D93818 (LangRef) Would it be desirable if I land all of these at once as well as this (93586) when

[PATCH] D93586: [InstCombine] use poison as placeholder for undemanded elems

2020-12-27 Thread Nikita Popov via Phabricator via cfe-commits
nikic accepted this revision. nikic added a comment. This revision is now accepted and ready to land. LGTM Comment at: llvm/lib/Transforms/InstCombine/InstCombineSimplifyDemanded.cpp:1073 for (unsigned i = 0; i != VWidth; ++i) { if (!DemandedElts[i]) { // If not d

[PATCH] D93586: [InstCombine] use poison as placeholder for undemanded elems

2020-12-25 Thread Juneyoung Lee via Phabricator via cfe-commits
aqjune added a comment. I'll make two more patches - the instsimplify/vectorizer/ changes that make insertelement poison, and the langref update to shufflevector. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93586/new/ https://reviews.llvm.or

[PATCH] D93586: [InstCombine] use poison as placeholder for undemanded elems

2020-12-23 Thread Juneyoung Lee via Phabricator via cfe-commits
aqjune added a comment. In D93586#2468350 , @spatel wrote: > It would be good to update those for consistency; the codegen tests are > supposed to be representative of what comes out of the IR optimizer. IIUC, we > could do the substitution on those file

[PATCH] D93586: [InstCombine] use poison as placeholder for undemanded elems

2020-12-22 Thread Sanjay Patel via Phabricator via cfe-commits
spatel added a comment. In D93586#2467248 , @aqjune wrote: >> There are 792 files in llvm/test, most of which are in test/Transform (119) >> and test/CodeGen(655). >> The transition will be swiftly done (if there's no other issue hopefully) by >> the nex

[PATCH] D93586: [InstCombine] use poison as placeholder for undemanded elems

2020-12-22 Thread Juneyoung Lee via Phabricator via cfe-commits
aqjune added a comment. In D93586#2467844 , @RKSimon wrote: > I'm sorry I've only just started looking at this - are you saying that you > want to handle all "insertelement undef" cases in one go and not just a > series of patcches after this one? It wi

[PATCH] D93586: [InstCombine] use poison as placeholder for undemanded elems

2020-12-22 Thread Simon Pilgrim via Phabricator via cfe-commits
RKSimon added a comment. I'm sorry I've only just started looking at this - are you saying that you want to handle all "insertelement undef" cases in one go and not just a series of patcches after this one? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm

[PATCH] D93586: [InstCombine] use poison as placeholder for undemanded elems

2020-12-21 Thread Juneyoung Lee via Phabricator via cfe-commits
aqjune added a comment. > There are 792 files in llvm/test, most of which are in test/Transform (119) > and test/CodeGen(655). > The transition will be swiftly done (if there's no other issue hopefully) by > the next weekend. Thinking about these again, do we need to make a poison copy for test

[PATCH] D93586: [InstCombine] use poison as placeholder for undemanded elems

2020-12-21 Thread Juneyoung Lee via Phabricator via cfe-commits
aqjune added a comment. In D93586#2466284 , @aqjune wrote: > The transition will be swiftly done (if there's no other issue hopefully) by > the next weekend. Oops, I meant this weekend hopefully. Repository: rG LLVM Github Monorepo CHANGES SINCE LAS

[PATCH] D93586: [InstCombine] use poison as placeholder for undemanded elems

2020-12-21 Thread Juneyoung Lee via Phabricator via cfe-commits
aqjune added a comment. In D93586#2464841 , @spatel wrote: > Besides changing IRBuilder and shufflevector's definition, I think we'll also > need updates in the vectorizers, InstSimplify, and other places in > InstCombine that use UndefValue for InsertEl

[PATCH] D93586: [InstCombine] use poison as placeholder for undemanded elems

2020-12-20 Thread Sanjay Patel via Phabricator via cfe-commits
spatel added reviewers: nlopes, regehr, RKSimon, zhengyangl, nikic, hfinkel. spatel added a comment. Thank you for working on this! Looking back at the bug comments (and adding reviewers based on those comments), this is a step towards killing undef that has been discussed for a long time now. :