[gem5-dev] Re: Upstreaming power-gem5

[Sandipan Das via gem5-dev]

Hi Boris, Gabe,

I've just uploaded patchset 4 with these changes:

[1] switch back to the use of INTREG_* for the special-purpose registers
https://gem5-review.googlesource.com/c/public/gem5/+/40882/
[2] rebase on top of the latest "develop" branch
[3] use of arch/power/regs/*.h as a result of the rebase

Please have a look.

As for [1], as promised, I will revisit it once I have a clear
view of how all the SPRs in question are read or modified by
different instructions.


- Sandipan
___
gem5-dev mailing list -- gem5-dev@gem5.org
To unsubscribe send an email to gem5-dev-le...@gem5.org
%(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s

[gem5-dev] Re: Upstreaming power-gem5

[Sandipan Das via gem5-dev]

Hi Boris,

Sure. For now, I can be the assignee. While I will be helping throughout
the process of upstreaming power-gem5, I might not be the one to submit
or follow up for all of the full system support patches that will come in
later. So, when required, I will reassign this accordingly.

- Sandipan

On 21/04/21 1:58 pm, Boris Shingarov wrote:
> Perfect. In accord with the latest email from Jason, I created GEM5-959: 
> https://gem5.atlassian.net/browse/GEM5-959 but I left the "Assignee" field 
> blank 
> as I will leave the choice to you whether you prefer to have your or my name 
> in 
> that field. ZjQcmQRYFpfptBannerStart
> This Message Is From an External Sender
> This message came from outside your organization.
> ZjQcmQRYFpfptBannerEnd
> Perfect.
> In accord with the latest email from Jason, I created GEM5-959:
> https://gem5.atlassian.net/browse/GEM5-959 
> <https://gem5.atlassian.net/browse/GEM5-959>
> but I left the "Assignee" field blank as I will leave the choice to you 
> whether 
> you prefer to have your or my name in that field.
> 
> -"Sandipan Das via gem5-dev"  <mailto:gem5-dev@gem5.org>> 
> wrote: -
> To: "Boris Shingarov" mailto:shinga...@labware.com>>
> From: "Sandipan Das via gem5-dev"  <mailto:gem5-dev@gem5.org>>
> Date: 04/19/2021 09:09AM
> Cc: "gem5 Developer List" mailto:gem5-dev@gem5.org>>, 
> "Sandipan Das" mailto:sandi...@linux.ibm.com>>
> Subject: [gem5-dev] Re: Upstreaming power-gem5
> 
> Hi Boris,
> 
> On 14/04/21 11:43 pm, Boris Shingarov wrote:
>  > Hi Sandipan,
>  >
>  > I notice some of the commits (which were, if not blocking reviewing other
>  > commits, but at least making comprehension of the whole body of commits 
> harder
>  > for me) are ready for merge, for example
>  > 
> https://gem5-review.googlesource.com/c/public/gem5/+/42943 
> <https://gem5-review.googlesource.com/c/public/gem5/+/42943> 
> 
>  > 
> <https://gem5-review.googlesource.com/c/public/gem5/+/42943 
> <https://gem5-review.googlesource.com/c/public/gem5/+/42943> >
>  >
>  > Do you want to start merging (what Gerrit calls "submit") them?
>  >
> 
> Sure. Sorry I could not respond earlier as I was on vacation but going 
> forward,
> I'll keep that in mind. Thanks for merging a few of the initial patches.
> 
> 
> - Sandipan
> ___
> gem5-dev mailing list -- gem5-dev@gem5.org <mailto:gem5-dev@gem5.org>
> To unsubscribe send an email to gem5-dev-le...@gem5.org 
> <mailto:gem5-dev-le...@gem5.org>
> %(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s
> 
___
gem5-dev mailing list -- gem5-dev@gem5.org
To unsubscribe send an email to gem5-dev-le...@gem5.org
%(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s

[gem5-dev] Re: Upstreaming power-gem5

[Sandipan Das via gem5-dev]

Hi Boris,

On 14/04/21 11:43 pm, Boris Shingarov wrote:
> Hi Sandipan,
> 
> I notice some of the commits (which were, if not blocking reviewing other 
> commits, but at least making comprehension of the whole body of commits 
> harder 
> for me) are ready for merge, for example
> https://gem5-review.googlesource.com/c/public/gem5/+/42943 
> 
> 
> Do you want to start merging (what Gerrit calls "submit") them?
> 

Sure. Sorry I could not respond earlier as I was on vacation but going forward,
I'll keep that in mind. Thanks for merging a few of the initial patches.


- Sandipan
___
gem5-dev mailing list -- gem5-dev@gem5.org
To unsubscribe send an email to gem5-dev-le...@gem5.org
%(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s

[gem5-dev] Re: Upstreaming power-gem5

[Sandipan Das via gem5-dev]

Hi Gabe,

On 25/02/21 1:10 pm, Gabe Black wrote:
> Hi Sandipan. You are correct, except that I would say you don't need to
> force push, just regular push. If I were at the head of a branch I wanted
> to (re)upload to gerrit, I would run:
> 
> git push origin HEAD:refs/for/develop
> 
> Gerrit will look at the Change-Id field in the commit message and use that
> to identify reviews. If one already exists, it will create a new patchset
> version (you can look at and compare them in the review UI if you like),
> and if it doesn't it will make a new review. In the output of the git
> command it will tell you which ones are new and which ones are updated if
> you're curious.
> 
> While it can be mildly disorienting clicking around a series where the
> ordering of reviews has changed (gerrit tries to go off of the patchset
> version you're currently looking at), it has no problem keeping track of
> everything.
> 

Great. Thanks!
I have made amends for most of the current review comments. As soon as
your register class related changes are merged, I will rebase my patches
and push them.

- Sandipan
___
gem5-dev mailing list -- gem5-dev@gem5.org
To unsubscribe send an email to gem5-dev-le...@gem5.org
%(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s

[gem5-dev] Re: Upstreaming power-gem5

[Sandipan Das via gem5-dev]

Hello Boris, Gabe,

I think I now have a good amount of changes to address from the initial
posting of the patch series. In case of mailing list based reviews, we
would typically post the whole series again with a V2 tag but I guess
Gerrit tracks changes based on Change-Id.

So as long as the Change-Id is preserved, force pushing the branch with
revised patches will upload the new revision to Gerrit while still
preserving all of the historical data such as review comments, etc.
Is this correct?

I am also planning to add a new patch that splits makeCRField() into
signed and unsigned variants (like Gabe suggested) and that would now
be the first patch of the series. Can that create any problems?


- Sandipan
___
gem5-dev mailing list -- gem5-dev@gem5.org
To unsubscribe send an email to gem5-dev-le...@gem5.org
%(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s

[gem5-dev] Re: Upstreaming power-gem5

[Sandipan Das via gem5-dev]

+CC: Luke

On 08/02/21 10:26 am, Sandipan Das wrote:
> Hello Boris, Gabe,
> 
> I have rebased and pushed the changes to gerrit.
> This is link to the first patch in the series:
> https://gem5-review.googlesource.com/c/public/gem5/+/40880
> 
> 
___
gem5-dev mailing list -- gem5-dev@gem5.org
To unsubscribe send an email to gem5-dev-le...@gem5.org
%(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s

[gem5-dev] Re: Upstreaming power-gem5

[Sandipan Das via gem5-dev]

Hello Gabe,

On 08/02/21 10:58 am, Gabe Black wrote:
> Thanks, I took a very quick pass through them, primarily looking for places
> touched outside of the arch/power directory (where the impact is larger).
> The only two things I saw were where you added an #if THE_ISA in the CPU
> (not ok, has to be handled differently), and adding some constants and
> checks to the object file class (looked pretty reasonable). The bulk of the
> changes are contained in arch/power which is great, since that should mean
> the constraints on how things are done should be reduced. I don't know the
> Power ISA at all, but I'll look at the changes from a generic gem5/ISA
> implementation perspective and give some feedback for the first few patches
> at some point in the near future.
> 

Thanks for going over them. I do agree with your comments on the first
patch and can wait until ZeroReg usage gets cleaned up across ISAs.
Looking forward to your inputs.


- Sandipan
___
gem5-dev mailing list -- gem5-dev@gem5.org
To unsubscribe send an email to gem5-dev-le...@gem5.org
%(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s

[gem5-dev] Re: Upstreaming power-gem5

[Sandipan Das via gem5-dev]

Hello Boris, Gabe,

I have rebased and pushed the changes to gerrit.
This is link to the first patch in the series:
https://gem5-review.googlesource.com/c/public/gem5/+/40880


- Sandipan
___
gem5-dev mailing list -- gem5-dev@gem5.org
To unsubscribe send an email to gem5-dev-le...@gem5.org
%(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s

[gem5-dev] Re: Upstreaming power-gem5

[Sandipan Das via gem5-dev]

On 05/02/21 6:06 pm, Gabe Black wrote:
> Re my commit, please feel free to add back the constants you need. I'm
> probably going to try splitting up the ISA constants consumed by non-ISA
> code from the ones that are internally used in registers.hh since I'm
> trying to get rid of the former, but if that same constant is useful
> *inside* an ISA then that's ok, I just want to keep them separated so one
> doesn't leak back into the other over time.
> 

Sure, Gabe. Thanks.


- Sandipan
___
gem5-dev mailing list -- gem5-dev@gem5.org
To unsubscribe send an email to gem5-dev-le...@gem5.org
%(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s

[gem5-dev] Re: Upstreaming power-gem5

[Sandipan Das via gem5-dev]

Hello Boris,

On 04/02/21 10:08 pm, Boris Shingarov wrote:
>  > The current sequence breaks 32-bit support in
>  > the beginning and then restores it back towards the end.
>  > Wondering if that could be a problem with the CI?
> 
> I would be surprised if there is even any POWER-specific CI at all.
> The one POWER binary we had (in test-progs), was removed at c1ebdf66f.  I've 
> been waiting on 86222736e (which just got in) before submitting
> https://gem5-review.googlesource.com/c/public/gem5/+/40635 
>  ,
> could you please code-review that?  Then, we are ready to merge your e52dbcb.

Done.

> 
>  > The current sequence breaks 32-bit support in
>  > the beginning and then restores it back towards the end.
> 
> Up to you really.  My guess is that when you look at how much `develop` has 
> diverged in the past months, you will find keeping the sequence less of a 
> thing.
> 

Got it. I started rebasing against 'develop' last night and ran
into a problem with one of the branch instruction related patches.
Hopefully, I can get that fixed soon. I also noticed this recent
patch from Gabe:

commit 7bb456f02
Author: Gabe Black 
Date:   Sun Jan 24 23:16:43 2021 -0800

arch-power: Delete unused register related constants.

I was actually planning on reusing some of these constants
like here:
https://github.com/sandip4n/gem5/commit/6ffda6f4583054a9d49b90cbd67189c813bd4dae

Maybe I'll add a patch that reintroduces the relevant ones.


- Sandipan
___
gem5-dev mailing list -- gem5-dev@gem5.org
To unsubscribe send an email to gem5-dev-le...@gem5.org
%(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s

[gem5-dev] Re: Upstreaming power-gem5

[Sandipan Das via gem5-dev]

Hello Boris,

On 04/02/21 12:43 am, Boris Shingarov wrote:
>> I think I had come across that problem too but I am sure
>> that one of my patches will fix that. Probably this one
> 
> Yes -- that's what I meant by "commits related to 3dd04381".
> So, let's start with this small area.
> 

Sure.

>> Yes, I can submit it via gerrit.
>> As a kernel developer, I am more used to mailing list based reviews
>> but feel free to let me know what works best for you.
> 
> Gerrit is the procedure currently used by the gem5 community.  Even though I 
> personally find it non-ideal, it is kind of a given for the foreseeable 
> future, and I think the optimal scenario (within the realistic choices) would 
> be if you started upstreaming using that procedure.  The other alternative, 
> of which I was afraid before I initially wrote to you, would have been if you 
> had abandoned the project or had no time/energy to do the rebasing / pushing 
> / working with the review, in that case I was thinking about just taking your 
> patches and putting them on Gerrit myself but I can see a whole number of 
> reasons to avoid this.
> 
> 

Sure, I'll submit the changes via Gerrit.
Aside from rebasing on top of the develop branch, I think it will
be easier for us if I bring all the 32-bit cleanups and fixes to
the beginning of the series and then introduce 64-bit mode followed
by new instructions. The current sequence breaks 32-bit support in
the beginning and then restores it back towards the end. Wondering
if that could be a problem with the CI?


- Sandipan
___
gem5-dev mailing list -- gem5-dev@gem5.org
To unsubscribe send an email to gem5-dev-le...@gem5.org
%(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s

[gem5-dev] Re: Upstreaming power-gem5

[Sandipan Das via gem5-dev]

On 03/02/21 11:59 pm, Sandipan Das wrote:
> Hello Boris,
> 
> On 03/02/21 2:34 am, Boris Shingarov wrote:
>> Hi Sandipan,
>>
>>> This makes it possible
>>> to run both 32-bit and 64-bit big and little endian PowerPC binaries
>>> in SE mode. If its okay with you, we can start by working on trying to
>>> get these changes reviewed and merged first.
>>
>> Yes, yes!  This aligns very well with both what sequence I think is the most 
>> realistic technically, and with my project's priorities.
>>
> 
> Great!
> 
>>> You can find them here: [develop-power]
>>
>> Some of these commits just scream as the perfect first candidates, for 
>> example 
>> https://github.com/sandip4n/gem5/commit/3dd0438159d67fe2b5bcc777b43046fdca90f6b9
>>  and related.  Because right now the Decoder is in a really sad state, 
>> crashing even on simple, perfectly valid programs (and that's even within 
>> BE, 32-bit SE), see for example https://gem5.atlassian.net/browse/GEM5-819.  
>> So, how about we start from that one?  How would you like to proceed -- do 
>> you want to submit on Gerrit using the regular process and I will review it?
>>
> 
> I think I had come across that problem too but I am sure
> that one of my patches will fix that. Probably this one:
> https://github.com/sandip4n/gem5/commit/3b0203e63d892c94fd0e67cfe79f5e6662504c4a
> 

Sorry I missed the other question. Yes, I can submit it via gerrit.
As a kernel developer, I am more used to mailing list based reviews
but feel free to let me know what works best for you.

That reminds me, I'll also need to rebase my code against the latest
"develop" branch.


- Sandipan
___
gem5-dev mailing list -- gem5-dev@gem5.org
To unsubscribe send an email to gem5-dev-le...@gem5.org
%(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s

[gem5-dev] Re: Upstreaming power-gem5

[Sandipan Das via gem5-dev]

Hello Boris,

On 03/02/21 2:34 am, Boris Shingarov wrote:
> Hi Sandipan,
> 
>> This makes it possible
>> to run both 32-bit and 64-bit big and little endian PowerPC binaries
>> in SE mode. If its okay with you, we can start by working on trying to
>> get these changes reviewed and merged first.
> 
> Yes, yes!  This aligns very well with both what sequence I think is the most 
> realistic technically, and with my project's priorities.
> 

Great!

>> You can find them here: [develop-power]
> 
> Some of these commits just scream as the perfect first candidates, for 
> example 
> https://github.com/sandip4n/gem5/commit/3dd0438159d67fe2b5bcc777b43046fdca90f6b9
>  and related.  Because right now the Decoder is in a really sad state, 
> crashing even on simple, perfectly valid programs (and that's even within BE, 
> 32-bit SE), see for example https://gem5.atlassian.net/browse/GEM5-819.  So, 
> how about we start from that one?  How would you like to proceed -- do you 
> want to submit on Gerrit using the regular process and I will review it?
> 

I think I had come across that problem too but I am sure
that one of my patches will fix that. Probably this one:
https://github.com/sandip4n/gem5/commit/3b0203e63d892c94fd0e67cfe79f5e6662504c4a

>> this has been a hobby/side project and hence, the biggest
>> challenge for us has been finding spare cycles
> 
> At my day job at LabWare, I write JIT compilers, and the development 
> technology is centered around simulation -- so I can't draw a bright line 
> where gem5 stops being a "side" project and becomes part of the "main" 
> project because the latter depends on it.  But it does mean that I naturally 
> have more timeslices for some aspects of gem5 than for other aspects.
>>> So, in order for external UARTs like 8250 to be
>> used for console interactions, we currently use some hacks
>> which I am sure are not quite upstreamable
> 
> Yeah, we are in the same boat here.
> I, too, have a long queue of hacks to gem5 which I did just to enable some 
> experiments with the JIT compiler; they've been waiting *years* that I rework 
> them into "real" patches to go upstream -- but that stage of work is very 
> slow and painful labour.
> 

Sounds interesting. I have minimal experience with JIT
compilers having worked on the Linux kernel's in-built
eBPF JIT compilers. Have peeked into Qemu's TCG too.

On that note, are there any plans to add a JIT engine
to gem5? Since gem5 supports checkpoints, uninteresting
parts of a program can be run in JITed mode to speed
things up.

>> The FS mode changes depend on 64-bit support but aside from that,
>> it also needs a fair amount
> 
> What scares me every time I think about FS, is that it will need coordinating 
> across projects/communities (Kernel, OPAL, ...) where I don't have a lot of 
> background in.  Well, let's worry about FS when the time comes.
> 

The folks that I work with here at IBM LTC can assist us
with that. Besides, like we agreed, lets start with SE
mode.


- Sandipan
___
gem5-dev mailing list -- gem5-dev@gem5.org
To unsubscribe send an email to gem5-dev-le...@gem5.org
%(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s

[gem5-dev] Re: Upstreaming power-gem5

[Sandipan Das via gem5-dev]

Hello Boris,

On 02/02/21 12:16 am, Boris Shingarov wrote:
> Dear Basavaraj, Sandipan and other contributors to power-gem5:
> 
> I am currently the maintainer of arch-power in gem5 and I am interested in 
> upstreaming power-gem5 to the mainline gem5.  My understanding from your 

Glad to hear that!

> OpenPOWER Summit presentation is that you intend to upstream it.  So I am 
> curious where we are with that.  What I am trying to avoid is duplication of 
> discovery of things that perhaps you already discovered.  E.g., maybe you 
> bumped 
> into some insurmountable difficulty which makes continuing not worth it -- in 
> that case I don't want to spend 6 months just to step on the same rake.  
> Therefore: could you shed some light on the project's current status?
> 

The branch (https://github.com/power-gem5/gem5/tree/gem5-experimental)
where we have all of the changes that make FS mode support possible for
the Power ISA is fairly out-of-date, at least by a couple of years now.
Since there are quite a few patches, we would like them to be reviewed
and merged in phases.

We began by adding 64-bit instruction support and I do try to rebase
and maintain this specific subset of patches. This makes it possible
to run both 32-bit and 64-bit big and little endian PowerPC binaries
in SE mode. If its okay with you, we can start by working on trying to
get these changes reviewed and merged first. You can find them here:
https://github.com/sandip4n/gem5/tree/develop-power

The FS mode changes depend on 64-bit support but aside from that, it
also needs a fair amount of cleanup in order to meet the patch submission
requirements. We can again split this into multiple phases based on the
sequence in which MMU, interrupts and OPAL firmware + Linux kernel support
get added. That being said, there are bits like external interrupt support
that requires us to add a supported interrupt controller, such as XICS
or XIVE, which are still missing. So, in order for external UARTs like
8250 to be used for console interactions, we currently use some hacks
which I am sure are not quite upstreamable.

I did write a console driver for OPAL firmware that allows the Linux
kernel to use an alternative firmware-provided console device but that
has not been merged either. Plus there are other quirks in OPAL firmware
that will need to go in since gem5 will not support any form of power
management (sleep states, etc.). These changes can be found here:
https://github.com/sandip4n/skiboot/commits/gem5-devel

For those of us at IBM Linux Technology Center (LTC), this has been a
hobby/side project and hence, the biggest challenge for us has been
finding spare cycles to work on improving, cleaning up and keeping things
up-to-date. We still absolutely intend to get these merged.


- Sandipan
___
gem5-dev mailing list -- gem5-dev@gem5.org
To unsubscribe send an email to gem5-dev-le...@gem5.org
%(web_page_url)slistinfo%(cgiext)s/%(_internal_name)s