Mailing-List: contact dev-help@impala.incubator.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@impala.incubator.apache.org
MIME-Version: 1.0
In-Reply-To: 
 <CAEoRBezskb7ZDH0Zn=F5NUkj1U4suJoVZw-cFAVaMmn9gdFZFA@mail.gmail.com>
References: <22f40c9f-d578-4b81-9f87-683857166805@cloudera.org>
 <04556600-a84b-43fc-b566-3756616b7eb7@cloudera.org>
 <CAKGvSBjQyWwJYd-crBb+TiCxzOFBzoNT8VUG340=AfM9i196+Q@mail.gmail.com>
 <CADZba-TwhQH0wtxP8aSVp2Ap_-PenP4Rd3z-5c2Naxcyw2ZK9g@mail.gmail.com>
 <CADY20s6bSUU=5PQvSEw5ZvtDk2Ci6HcXtY-Wq_eKdwJCyk5V5A@mail.gmail.com>
 <201602181316.u1IDGZmR015375@d03av01.boulder.ibm.com>
 <CADY20s5+EnfG+B22Ct7RTZqSq3SkYNV74-ePJHjGifkzSzxpzw@mail.gmail.com>
 <201602191036.u1JAaqln029510@d01av04.pok.ibm.com>
 <CAEoRBezskb7ZDH0Zn=F5NUkj1U4suJoVZw-cFAVaMmn9gdFZFA@mail.gmail.com>
From: Todd Lipcon <todd@cloudera.com>
Date: Tue, 23 Feb 2016 16:04:56 -0800
Message-ID: 
 <CADY20s4KW4zSTYT82Z2P=qDL6fM0yHYL46g0HC-+Ni8ozLXk-A@mail.gmail.com>
Subject: Re: Passing DataLayout to Module/Function Pass Manager of LLVM
To: Tim Armstrong <tarmstrong@cloudera.com>
Cc: dev@impala.incubator.apache.org,
 nishidha panpaliya <nishidha27@gmail.com>,
	Silvius Rus <srus@cloudera.com>, Sudarshan Jagadale <jagadale@us.ibm.com>
Content-Type: multipart/alternative; boundary=001a1140711c1cef51052c78d27f

--001a1140711c1cef51052c78d27f
Content-Type: text/plain; charset=UTF-8

You might consider skipping 3.7 and going right to 3.8. It's in its second
release candidate and should be released in the next week or so.

-Todd

On Tue, Feb 23, 2016 at 3:52 PM, Tim Armstrong <tarmstrong@cloudera.com>
wrote:

> 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 <nishidha@us.ibm.com>
> 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 <nishidha@us.ibm.com> wrote:
>>
>> From: Todd Lipcon <todd@cloudera.com>
>> To: Nishidha Panpaliya/Austin/Contr/IBM@IBMUS
>> Cc: dev@impala.incubator.apache.org, nishidha panpaliya <
>> nishidha27@gmail.com>, Silvius Rus <srus@cloudera.com>, 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 <
>> *nishidha@us.ibm.com* <nishidha@us.ibm.com>> 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 <*todd@cloudera.com* <todd@cloudera.com>>
>>    To: *dev@impala.incubator.apache.org*
>>    <dev@impala.incubator.apache.org>
>>    Cc: Silvius Rus <*srus@cloudera.com* <srus@cloudera.com>>, nishidha
>>    panpaliya <*nishidha27@gmail.com* <nishidha27@gmail.com>>, 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 <*henry@cloudera.com*
>>    <henry@cloudera.com>> wrote:
>>    > (Moving this conversation to *dev@impala.incubator.apache.org*
>>    <dev@impala.incubator.apache.org>,
>>    > *impala-dev@cloudera.org* <impala-dev@cloudera.org> to bcc:)
>>    >
>>    > On 17 February 2016 at 09:13, Silvius Rus <*srus@cloudera.com*
>>    <srus@cloudera.com>> 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 <
>>    *nishidha27@gmail.com* <nishidha27@gmail.com>>
>>    >> 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 *impala-dev+unsubscribe@cloudera.org*
>>    <impala-dev%2Bunsubscribe@cloudera.org>.
>>    >>>
>>    >>
>>    >> --
>>    >> 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 *impala-dev+unsubscribe@cloudera.org*
>>    <impala-dev%2Bunsubscribe@cloudera.org>.
>>    >>
>>    >
>>    >
>>    >
>>    > --
>>    > Henry Robinson
>>    > Software Engineer
>>    > Cloudera
>>    > *415-994-6679* <415-994-6679>
>>
>>
>>
>>    --
>>    Todd Lipcon
>>    Software Engineer, Cloudera
>>
>>
>>
>>
>>
>>
>>
>> --
>> Todd Lipcon
>> Software Engineer, Cloudera
>>
>>
>


-- 
Todd Lipcon
Software Engineer, Cloudera

--001a1140711c1cef51052c78d27f--