Sean Silva via llvm-dev <llvm-dev at lists.llvm.org> writes:> No tests fail with the patch below, so I would say it's pretty useless. It > seems that the C bindings are the only user but we can probably just have them > return IPSCCP instead.I don't necessarily think your conclusion is wrong, but the patch isn't proving what you think it's proving. In fact, the below passes all tests as well. When you call passes through `opt` they don't end up calling through the createXYZPass path. diff --git a/lib/Transforms/IPO/IPConstantPropagation.cpp b/lib/Transforms/IPO/IPConstantPropagation.cpp index b3ee499..8d70b98 100644 --- a/lib/Transforms/IPO/IPConstantPropagation.cpp +++ b/lib/Transforms/IPO/IPConstantPropagation.cpp @@ -253,7 +253,9 @@ char IPCP::ID = 0; INITIALIZE_PASS(IPCP, "ipconstprop", "Interprocedural constant propagation", false, false) -ModulePass *llvm::createIPConstantPropagationPass() { return new IPCP(); } +ModulePass *llvm::createIPConstantPropagationPass() { + llvm_unreachable("fnord"); +} bool IPCP::runOnModule(Module &M) { if (skipModule(M))> Any idea why this wasn't removed when IPSCCP was introduced? Probably worth > understanding that before ripping it out, but in the current state of things I > don't think removing it will be problematic. > > diff --git a/llvm/lib/Transforms/IPO/IPConstantPropagation.cpp b/llvm/lib/ > Transforms/IPO/IPConstantPropagation.cpp > index a1533b3..24aea5c 100644 > --- a/llvm/lib/Transforms/IPO/IPConstantPropagation.cpp > +++ b/llvm/lib/Transforms/IPO/IPConstantPropagation.cpp > @@ -51,7 +51,9 @@ char IPCP::ID = 0; > INITIALIZE_PASS(IPCP, "ipconstprop", > "Interprocedural constant propagation", false, false) > > -ModulePass *llvm::createIPConstantPropagationPass() { return new IPCP(); } > +ModulePass *llvm::createIPConstantPropagationPass() { > + return createIPSCCPPass(); > +} > > bool IPCP::runOnModule(Module &M) { > if (skipModule(M)) > > -- Sean Silva > > On Tue, May 3, 2016 at 3:06 PM, Davide Italiano via llvm-dev < > llvm-dev at lists.llvm.org> wrote: > > The pass is pretty rudimental (as the comment at the top of the file > hints), and it seems LLVM already has IPSCCP (which should do a better > job at interprocedural constant propagation). > I'm also not entirely sure it's used anywhere. > Is there any reason to keep it around? > > Thanks, > > -- > Davide > > "There are no solved problems; there are only problems that are more > or less solved" -- Henri Poincare > _______________________________________________ > LLVM Developers mailing list > llvm-dev at lists.llvm.org > http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev > > _______________________________________________ > LLVM Developers mailing list > llvm-dev at lists.llvm.org > http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
On Tue, May 3, 2016 at 11:01 PM, Justin Bogner <mail at justinbogner.com> wrote:> Sean Silva via llvm-dev <llvm-dev at lists.llvm.org> writes: >> No tests fail with the patch below, so I would say it's pretty useless. It >> seems that the C bindings are the only user but we can probably just have them >> return IPSCCP instead. > > I don't necessarily think your conclusion is wrong, but the patch isn't > proving what you think it's proving. In fact, the below passes all tests > as well. When you call passes through `opt` they don't end up calling > through the createXYZPass path. > > diff --git a/lib/Transforms/IPO/IPConstantPropagation.cpp b/lib/Transforms/IPO/IPConstantPropagation.cpp > index b3ee499..8d70b98 100644 > --- a/lib/Transforms/IPO/IPConstantPropagation.cpp > +++ b/lib/Transforms/IPO/IPConstantPropagation.cpp > @@ -253,7 +253,9 @@ char IPCP::ID = 0; > INITIALIZE_PASS(IPCP, "ipconstprop", > "Interprocedural constant propagation", false, false) > > -ModulePass *llvm::createIPConstantPropagationPass() { return new IPCP(); } > +ModulePass *llvm::createIPConstantPropagationPass() { > + llvm_unreachable("fnord"); > +} >I think this should do the job: $ grep -R 'ipconstprop' * | wc -l 7 $ sed -i.bak 's/ipconstprop/ipsccp/' * $ grep -R 'ipconstprop' * | wc -l 0 and two tests are actually failing :| Testing Time: 0.10s ******************** Failing Tests (2): LLVM :: Transforms/IPConstantProp/return-argument.ll LLVM :: Transforms/IPConstantProp/return-constants.ll Expected Passes : 11 Unexpected Failures: 2 I'll take a closer look tomorrow.> bool IPCP::runOnModule(Module &M) { > if (skipModule(M)) > >> Any idea why this wasn't removed when IPSCCP was introduced? Probably worth >> understanding that before ripping it out, but in the current state of things I >> don't think removing it will be problematic. >> >> diff --git a/llvm/lib/Transforms/IPO/IPConstantPropagation.cpp b/llvm/lib/ >> Transforms/IPO/IPConstantPropagation.cpp >> index a1533b3..24aea5c 100644 >> --- a/llvm/lib/Transforms/IPO/IPConstantPropagation.cpp >> +++ b/llvm/lib/Transforms/IPO/IPConstantPropagation.cpp >> @@ -51,7 +51,9 @@ char IPCP::ID = 0; >> INITIALIZE_PASS(IPCP, "ipconstprop", >> "Interprocedural constant propagation", false, false) >> >> -ModulePass *llvm::createIPConstantPropagationPass() { return new IPCP(); } >> +ModulePass *llvm::createIPConstantPropagationPass() { >> + return createIPSCCPPass(); >> +} >> >> bool IPCP::runOnModule(Module &M) { >> if (skipModule(M)) >> >> -- Sean Silva >> >> On Tue, May 3, 2016 at 3:06 PM, Davide Italiano via llvm-dev < >> llvm-dev at lists.llvm.org> wrote: >> >> The pass is pretty rudimental (as the comment at the top of the file >> hints), and it seems LLVM already has IPSCCP (which should do a better >> job at interprocedural constant propagation). >> I'm also not entirely sure it's used anywhere. >> Is there any reason to keep it around? >> >> Thanks, >> >> -- >> Davide >> >> "There are no solved problems; there are only problems that are more >> or less solved" -- Henri Poincare >> _______________________________________________ >> LLVM Developers mailing list >> llvm-dev at lists.llvm.org >> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev >> >> _______________________________________________ >> LLVM Developers mailing list >> llvm-dev at lists.llvm.org >> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev-- Davide "There are no solved problems; there are only problems that are more or less solved" -- Henri Poincare
On Tue, May 3, 2016 at 11:52 PM, Davide Italiano <davide at freebsd.org> wrote:> On Tue, May 3, 2016 at 11:01 PM, Justin Bogner <mail at justinbogner.com> wrote: >> Sean Silva via llvm-dev <llvm-dev at lists.llvm.org> writes: >>> No tests fail with the patch below, so I would say it's pretty useless. It >>> seems that the C bindings are the only user but we can probably just have them >>> return IPSCCP instead. >> >> I don't necessarily think your conclusion is wrong, but the patch isn't >> proving what you think it's proving. In fact, the below passes all tests >> as well. When you call passes through `opt` they don't end up calling >> through the createXYZPass path. >> >> diff --git a/lib/Transforms/IPO/IPConstantPropagation.cpp b/lib/Transforms/IPO/IPConstantPropagation.cpp >> index b3ee499..8d70b98 100644 >> --- a/lib/Transforms/IPO/IPConstantPropagation.cpp >> +++ b/lib/Transforms/IPO/IPConstantPropagation.cpp >> @@ -253,7 +253,9 @@ char IPCP::ID = 0; >> INITIALIZE_PASS(IPCP, "ipconstprop", >> "Interprocedural constant propagation", false, false) >> >> -ModulePass *llvm::createIPConstantPropagationPass() { return new IPCP(); } >> +ModulePass *llvm::createIPConstantPropagationPass() { >> + llvm_unreachable("fnord"); >> +} >> > > I think this should do the job: > > $ grep -R 'ipconstprop' * | wc -l > 7 > $ sed -i.bak 's/ipconstprop/ipsccp/' * > $ grep -R 'ipconstprop' * | wc -l > 0 > > and two tests are actually failing :| > > Testing Time: 0.10s > ******************** > Failing Tests (2): > LLVM :: Transforms/IPConstantProp/return-argument.ll > LLVM :: Transforms/IPConstantProp/return-constants.ll > > Expected Passes : 11 > Unexpected Failures: 2 > > I'll take a closer look tomorrow. > >I analyzed this and realized there are some cases that are actually not handled properly by SCCP but they are by the naive algorithm. For example, in return-argument SCCP doesn't propagate the return value correctly -- Davide "There are no solved problems; there are only problems that are more or less solved" -- Henri Poincare
On Tue, May 3, 2016 at 11:01 PM, Justin Bogner <mail at justinbogner.com> wrote:> Sean Silva via llvm-dev <llvm-dev at lists.llvm.org> writes: > > No tests fail with the patch below, so I would say it's pretty useless. > It > > seems that the C bindings are the only user but we can probably just > have them > > return IPSCCP instead. > > I don't necessarily think your conclusion is wrong, but the patch isn't > proving what you think it's proving. In fact, the below passes all tests > as well. When you call passes through `opt` they don't end up calling > through the createXYZPass path. >Ah, interesting! Thanks for catching this! -- Sean Silva> > diff --git a/lib/Transforms/IPO/IPConstantPropagation.cpp > b/lib/Transforms/IPO/IPConstantPropagation.cpp > index b3ee499..8d70b98 100644 > --- a/lib/Transforms/IPO/IPConstantPropagation.cpp > +++ b/lib/Transforms/IPO/IPConstantPropagation.cpp > @@ -253,7 +253,9 @@ char IPCP::ID = 0; > INITIALIZE_PASS(IPCP, "ipconstprop", > "Interprocedural constant propagation", false, false) > > -ModulePass *llvm::createIPConstantPropagationPass() { return new IPCP(); } > +ModulePass *llvm::createIPConstantPropagationPass() { > + llvm_unreachable("fnord"); > +} > > bool IPCP::runOnModule(Module &M) { > if (skipModule(M)) > > > Any idea why this wasn't removed when IPSCCP was introduced? Probably > worth > > understanding that before ripping it out, but in the current state of > things I > > don't think removing it will be problematic. > > > > diff --git a/llvm/lib/Transforms/IPO/IPConstantPropagation.cpp > b/llvm/lib/ > > Transforms/IPO/IPConstantPropagation.cpp > > index a1533b3..24aea5c 100644 > > --- a/llvm/lib/Transforms/IPO/IPConstantPropagation.cpp > > +++ b/llvm/lib/Transforms/IPO/IPConstantPropagation.cpp > > @@ -51,7 +51,9 @@ char IPCP::ID = 0; > > INITIALIZE_PASS(IPCP, "ipconstprop", > > "Interprocedural constant propagation", false, false) > > > > -ModulePass *llvm::createIPConstantPropagationPass() { return new > IPCP(); } > > +ModulePass *llvm::createIPConstantPropagationPass() { > > + return createIPSCCPPass(); > > +} > > > > bool IPCP::runOnModule(Module &M) { > > if (skipModule(M)) > > > > -- Sean Silva > > > > On Tue, May 3, 2016 at 3:06 PM, Davide Italiano via llvm-dev < > > llvm-dev at lists.llvm.org> wrote: > > > > The pass is pretty rudimental (as the comment at the top of the file > > hints), and it seems LLVM already has IPSCCP (which should do a > better > > job at interprocedural constant propagation). > > I'm also not entirely sure it's used anywhere. > > Is there any reason to keep it around? > > > > Thanks, > > > > -- > > Davide > > > > "There are no solved problems; there are only problems that are more > > or less solved" -- Henri Poincare > > _______________________________________________ > > LLVM Developers mailing list > > llvm-dev at lists.llvm.org > > http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev > > > > _______________________________________________ > > LLVM Developers mailing list > > llvm-dev at lists.llvm.org > > http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev >-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20160504/65e1301b/attachment.html>