Hi Nishidha, I'm going to spend some time on the LLVM 3.7 upgrade, likely next week, once we finish up work on our next release. If you have a patch you are able to share now it would be interesting to compare.
It looks like I probably have a bit of work to get everything working correctly in LLVM 3.7 and to validate the performance. Cheers, TIm On Fri, Feb 19, 2016 at 2:36 AM, Nishidha Panpaliya <[email protected]> wrote: > Thanks Todd for this additional information. I'll definitely check this > once I'm done with porting Impala successfully. > > Thanks again. > Nishidha > > [image: Inactive hide details for Todd Lipcon ---02/19/2016 02:08:42 > AM---On Thu, Feb 18, 2016 at 5:16 AM, Nishidha Panpaliya <nishidha]Todd > Lipcon ---02/19/2016 02:08:42 AM---On Thu, Feb 18, 2016 at 5:16 AM, > Nishidha Panpaliya <[email protected]> wrote: > > From: Todd Lipcon <[email protected]> > To: Nishidha Panpaliya/Austin/Contr/IBM@IBMUS > Cc: [email protected], nishidha panpaliya < > [email protected]>, Silvius Rus <[email protected]>, Sudarshan > Jagadale/Austin/Contr/IBM@IBMUS > Date: 02/19/2016 02:08 AM > > Subject: Re: Passing DataLayout to Module/Function Pass Manager of LLVM > ------------------------------ > > > > On Thu, Feb 18, 2016 at 5:16 AM, Nishidha Panpaliya <*[email protected]* > <[email protected]>> wrote: > > Hi Todd, > > Thanks a lot. This really helped me. > > So, in case I can simply ignore module->setDataLayout(...) as it is > already set in it once. In my case, with LLVM 3.3, datalayout was added as > Pass to ModulePassManager and FunctionPassManager, which now is not needed, > as it is already set in the module object being used by these passmanagers. > Please confirm my understanding. > > Also, it seems in your module_builder.cc, you are continuing using > PassManagers which are made as legacy now in LLVM 3.7. I hope it is okay to > use those legacy PassManagers instead of new PassManagers in > IR/PassManager.h. > > > > I haven't followed this API change closely -- I didn't realize that > PassManagers are legacy now. It seems like our codegen is working, though :) > > One thing to be aware of is that some inlining heuristics or thresholds > changed with our upgrade. We had to do some patches to get the generated > code back to the original performance. So, make sure you benchmark > carefully and/or inspect the generated machine code for unwanted call > instructions. (in the Kudu code we have a -codegen_dump_mc flag to allow > dumping the asm, not sure if Impala has the same) > > -Todd > > > [image: Inactive hide details for Todd Lipcon ---02/17/2016 11:45:53 > PM---Regarding the DataLayout question, here's how we handle it wi]Todd > Lipcon ---02/17/2016 11:45:53 PM---Regarding the DataLayout question, > here's how we handle it with LLVM 3.7 in Kudu: > > From: Todd Lipcon <*[email protected]* <[email protected]>> > To: *[email protected]* <[email protected]> > Cc: Silvius Rus <*[email protected]* <[email protected]>>, nishidha > panpaliya <*[email protected]* <[email protected]>>, Nishidha > Panpaliya/Austin/Contr/IBM@IBMUS, Sudarshan > Jagadale/Austin/Contr/IBM@IBMUS > Date: 02/17/2016 11:45 PM > Subject: Re: Passing DataLayout to Module/Function Pass Manager of LLVM > ------------------------------ > > > > > Regarding the DataLayout question, here's how we handle it with LLVM > 3.7 in Kudu: > > > > *https://github.com/cloudera/kudu/blob/master/src/kudu/codegen/module_builder.cc#L267* > > <https://github.com/cloudera/kudu/blob/master/src/kudu/codegen/module_builder.cc#L267> > > You can see the diff in this mega commit: > > > *https://github.com/cloudera/kudu/commit/9806863e78107505a622b44112a897189d9b3c24* > > <https://github.com/cloudera/kudu/commit/9806863e78107505a622b44112a897189d9b3c24> > > On Wed, Feb 17, 2016 at 10:08 AM, Henry Robinson <*[email protected]* > <[email protected]>> wrote: > > (Moving this conversation to *[email protected]* > <[email protected]>, > > *[email protected]* <[email protected]> to bcc:) > > > > On 17 February 2016 at 09:13, Silvius Rus <*[email protected]* > <[email protected]>> wrote: > > > >> Hey Nishidha, > >> > >> Would you be interested in contributing the changes you've made so > far to > >> the Impala ASF project? It's easier to get help in context if you > post a > >> patch for review. Upgrading to LLVM 3.7 would be beneficial to the > project > >> in general, so I think others will jump in to help. > >> > >> *https://github.com/cloudera/Impala/wiki/Contributing-to-Impala* > <https://github.com/cloudera/Impala/wiki/Contributing-to-Impala> > >> > >> Silvius > >> > >> On Wed, Feb 17, 2016 at 5:00 AM, nishidha panpaliya < > *[email protected]* <[email protected]>> > >> wrote: > >> > >>> Alright Tim. Thanks for the reply. > >>> > >>> Regards, > >>> Nishidha > >>> > >>> > >>> On Tuesday, 16 February 2016 19:16:34 UTC+5:30, nishidha panpaliya > wrote: > >>>> > >>>> Hello, > >>>> > >>>> In Impala source code, I've encountered a place in > >>>> be/src/codegen/llvm-codegen.cc where DataLayout's pointer is > passed to > >>>> PassManager::addPass method. > >>>> > >>>> module_pass_manager->add(new DataLayout(data_layout_str)); > >>>> fn_pass_manager->add(new DataLayout(data_layout_str)); > >>>> > >>>> > >>>> *Note*: addPass method is also changed to add in LLVM 3.7. > >>>> > >>>> I wanted to understand what exactly these two lines are doing. > What > >>>> would be the impact if they are commented/removed? And if there > is any test > >>>> coverage for this? > >>>> > >>>> My rationale behind all these questions is to port Impala on > ppc64le, > >>>> I'd to upgrade LLVM 3.3 to 3.7. And in LLVM 3.7, DataLayout is > not derived > >>>> from class Pass and hence these lines do not compile with LLVM > 3.7. So, I > >>>> need to either find equivalent of this or have to remove. > >>>> > >>>> I'll be grateful to you if you could provide some insights here. > >>>> > >>>> Thanks in advance, > >>>> Nishidha > >>>> > >>>> -- > >>> You received this message because you are subscribed to the Google > Groups > >>> "Impala Dev" group. > >>> To unsubscribe from this group and stop receiving emails from it, > send an > >>> email to *[email protected]* > <impala-dev%[email protected]>. > >>> > >> > >> -- > >> You received this message because you are subscribed to the Google > Groups > >> "Impala Dev" group. > >> To unsubscribe from this group and stop receiving emails from it, > send an > >> email to *[email protected]* > <impala-dev%[email protected]>. > >> > > > > > > > > -- > > Henry Robinson > > Software Engineer > > Cloudera > > *415-994-6679* <415-994-6679> > > > > -- > Todd Lipcon > Software Engineer, Cloudera > > > > > > > > -- > Todd Lipcon > Software Engineer, Cloudera > >
