Re: [patch, sim] Add isel instruction

Re: [patch, sim] Add isel instruction

[Abid, Hafiz]

On 30/04/13 18:09:41, Mike Frysinger wrote:
> On Tuesday 30 April 2013 07:12:35 Abid, Hafiz wrote:
> > On 30/04/13 12:05:14, Joel Brobecker wrote:
> > > > Thansk for lettting me know. If this is urgent for you then I  
> can
> > > > revert as it may take me some time to find a fix.
> > >
> > > Do you have an idea of what "some time" would mean?
> >
> > By the end of this week.
> 
> i'm fine with that ... if it goes longer, then let's punt it
> -mike

It looks that isel was already supported in the simulator for the e500.  
The duplication caused the error that Joel observed. I will revert now  
with the following.

Thanks,
Abid

sim/ppc/ChangeLog:
2013-05-03  Hafiz Abid Qadeer  <abidh@codesourcery.com>

	revert:
	2013-04-19  Nathan Froyd  <froydnj@codesourcery.com>
	* ppc-instructions (isel): New instruction.

diff --git a/sim/ppc/ppc-instructions b/sim/ppc/ppc-instructions
index d76a765..1b8fd89 100644
--- a/sim/ppc/ppc-instructions
+++ b/sim/ppc/ppc-instructions
@@ -3455,19 +3455,6 @@  
void::function::invalid_zero_divide_operation:cpu *proces
         PPC_INSN_MFCR(RT_BITMASK);

  #
-# I.3.3.15 Fixed-Point Select
-#
-
-0.31,6.RT,11.RA,16.RB,21.BF,26.15,31./:A::isel:Integer Select
-       unsigned_word a;
-       if (RA_is_0) a = 0;
-       else         a = *rA;
-       if (CR & (1 << (31 - BF)))
-         *rT = a;
-       else
-         *rT = *rB;
-
-#

Re: [patch, sim] Add isel instruction

[Joel Brobecker]

> sim/ppc/ChangeLog:
> 2013-05-03  Hafiz Abid Qadeer  <abidh@codesourcery.com>
> 
> 	revert:
> 	2013-04-19  Nathan Froyd  <froydnj@codesourcery.com>
> 	* ppc-instructions (isel): New instruction.

Thanks!

For the record, I also moved the original ChangeLog entry from
sim/ChangeLog to sim/ppc/ChangeLog.

-- 
Joel

Re: [patch, sim] Add isel instruction

[Abid, Hafiz]

Hi Mike,
Thanks for quick response.

> erm, so you didn't write this patch, but Nathan Froyd did ?
Yes but he no longer works for Mentor Graphics. We have been carrying  
this patch for some time in our internal branches. I just put it in  
latest code and tested it.

Regards,
Abid

On 19/04/13 18:48:37, Mike Frysinger wrote:
> On Friday 19 April 2013 13:15:06 Abid, Hafiz wrote:
> > This patch adds support for isel instruction in the ppc simulator.
> > Tested manually by running a small assembly file through the  
> simulator.
> > Is this ok?
> 
> i don't really know anything about the ppc ISA, so i can't vet  
> changes on that
> level.  the style looks OK i guess.
> 
> > sim/ChangeLog:
> > 2013-04-19  Nathan Froyd  <froydnj@codesourcery.com>
> >
> > 	* ppc-instructions (isel): New instruction.
> 
> erm, so you didn't write this patch, but Nathan Froyd did ?
> -mike
> 

Re: [patch, sim] Add isel instruction

[Mike Frysinger]

[-- Attachment #1: Type: Text/Plain, Size: 471 bytes --]

On Friday 19 April 2013 13:52:41 Abid, Hafiz wrote:
> > erm, so you didn't write this patch, but Nathan Froyd did ?
> 
> Yes but he no longer works for Mentor Graphics. We have been carrying
> this patch for some time in our internal branches. I just put it in
> latest code and tested it.

it's probably a safe assumption you guys know a lot more about the powerpc ISA 
than myself.  i don't see anything wrong with the patch, so feel free to push 
it.
-mike

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[patch, sim] Add isel instruction

[Abid, Hafiz]

Hi,
This patch adds support for isel instruction in the ppc simulator.  
Tested manually by running a small assembly file through the simulator.  
Is this ok?

Regards,
Abid

sim/ChangeLog:
2013-04-19  Nathan Froyd  <froydnj@codesourcery.com>

	* ppc-instructions (isel): New instruction.

diff --git a/sim/ppc/ppc-instructions b/sim/ppc/ppc-instructions
index 1b8fd89..d76a765 100644
--- a/sim/ppc/ppc-instructions
+++ b/sim/ppc/ppc-instructions
@@ -3455,6 +3455,19 @@  
void::function::invalid_zero_divide_operation:cpu *processor,  
unsigned_word cia,
  	PPC_INSN_MFCR(RT_BITMASK);

  #
+# I.3.3.15 Fixed-Point Select
+#
+
+0.31,6.RT,11.RA,16.RB,21.BF,26.15,31./:A::isel:Integer Select
+	unsigned_word a;
+	if (RA_is_0) a = 0;
+	else         a = *rA;
+	if (CR & (1 << (31 - BF)))
+	  *rT = a;
+	else
+	  *rT = *rB;
+
+#
From gdb-patches-return-100695-listarch-gdb-patches=sources.redhat.com@sourceware.org Fri Apr 19 17:46:20 2013
Return-Path: <gdb-patches-return-100695-listarch-gdb-patches=sources.redhat.com@sourceware.org>
Delivered-To: listarch-gdb-patches@sources.redhat.com
Received: (qmail 7404 invoked by alias); 19 Apr 2013 17:46:20 -0000
Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm
Precedence: bulk
List-Id: <gdb-patches.sourceware.org>
List-Subscribe: <mailto:gdb-patches-subscribe@sourceware.org>
List-Archive: <http://sourceware.org/ml/gdb-patches/>
List-Post: <mailto:gdb-patches@sourceware.org>
List-Help: <mailto:gdb-patches-help@sourceware.org>, <http://sourceware.org/ml/#faqs>
Sender: gdb-patches-owner@sourceware.org
Delivered-To: mailing list gdb-patches@sourceware.org
Received: (qmail 7393 invoked by uid 89); 19 Apr 2013 17:46:20 -0000
X-Spam-SWARE-Status: No, score=-9.4 required=5.0 tests=AWL,BAYES_00,KHOP_PGP_SIGNED,KHOP_RCVD_UNTRUST,KHOP_THREADED,RCVD_IN_DNSWL_HI,RCVD_IN_HOSTKARMA_W,RP_MATCHES_RCVD autolearn=ham version=3.3.1
Received: from smtp.gentoo.org (HELO smtp.gentoo.org) (140.211.166.183)    by sourceware.org (qpsmtpd/0.84/v0.84-167-ge50287c) with ESMTP; Fri, 19 Apr 2013 17:46:19 +0000
Received: from vapier.localnet (localhost [127.0.0.1])	by smtp.gentoo.org (Postfix) with ESMTP id F3F3133DECE;	Fri, 19 Apr 2013 17:46:17 +0000 (UTC)
From: Mike Frysinger <vapier@gentoo.org>
To: "Abid, Hafiz" <hafiz_abid@mentor.com>, Nathan Froyd <froydnj@codesourcery.com>
Subject: Re: [patch, sim] Add isel instruction
Date: Fri, 19 Apr 2013 19:44:00 -0000
User-Agent: KMail/1.13.7 (Linux/3.8.3; KDE/4.6.5; x86_64; ; )
Cc: gdb-patches@sourceware.org
References: <1366391706.7941.1@abidh-ubunto1104>
In-Reply-To: <1366391706.7941.1@abidh-ubunto1104>
MIME-Version: 1.0
Content-Type: multipart/signed;  boundary="nextPart4646959.Ud0x7NujcZ";  protocol="application/pgp-signature";  micalg=pgp-sha1
Content-Transfer-Encoding: 7bit
Message-Id: <201304191348.39851.vapier@gentoo.org>
X-Virus-Found: No
X-SW-Source: 2013-04/txt/msg00621.txt.bz2


--nextPart4646959.Ud0x7NujcZ
Content-Type: Text/Plain;
  charset="iso-8859-1"
Content-Transfer-Encoding: quoted-printable
Content-length: 522

On Friday 19 April 2013 13:15:06 Abid, Hafiz wrote:
> This patch adds support for isel instruction in the ppc simulator.
> Tested manually by running a small assembly file through the simulator.
> Is this ok?

i don't really know anything about the ppc ISA, so i can't vet changes on that 
level.  the style looks OK i guess.

> sim/ChangeLog:
> 2013-04-19  Nathan Froyd  <froydnj@codesourcery.com>
> 
> 	* ppc-instructions (isel): New instruction.

erm, so you didn't write this patch, but Nathan Froyd did ?
-mike

--nextPart4646959.Ud0x7NujcZ
Content-Type: application/pgp-signature; name=signature.asc
Content-Description: This is a digitally signed message part.
Content-length: 836

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.17 (GNU/Linux)

iQIcBAABAgAGBQJRcYN3AAoJEEFjO5/oN/WBKFkP/jLqdlmNUnFnGj2+jFA9PTmn
EpBFcLIbftrdyvR95iaiGZHIzTLhMVKuCn/sqbYoGplvZy+SWSn5HuEidYFQiAs/
E9x0ndHbL+PtqoU4/nfUGYbHePtGjyJo7w71esy81NrbMnA7Jh6d8HLgda7x5/uK
McfFYr3JJiyUSnlpoh2URbPNvz4V8gCJ5Yj8V+NEOHsxC48DfFVxKYfJ4xcyGWEt
TvhpLW7oE8SBJxKC884iWk8M8lAL76DN6o1Phk39HcXPFqFKIhebU6pCRrCIzToG
zQOEKcxxe7zCvFKEbsvJELALSFTJ5SfWOvVPt7fecSL8UVuE4SPk2jRlHXcLq7go
H177W1Xt8ffDSfAOzIdBsOM1k6f7GfSYk5oT6AN61ipPdR9D2quLxeSHZdJVa9DN
Rfn+3a8bx7ejbJP/c5iRbtVTQJA10+G2dMa9qv5aE6vxgx2fl/aEm6TE7Rve9kqN
sfqG/PaHDmoQLdgk3F9MMpnGhSf7Ki9k9R4p9t9PBVuyLByEOILla+cVbu47gyHR
YiW2oIGF7VZIXtDTeB4fSiLhxu2tRInDYmP0FT5sY9ID3n1V44fGtJnC807x6TEe
2odRPQlLizqE4QJSYSKIQkUDytc4jXFzBXOVoOg2Sic85bsrYDjlhev6Bubx3aS/
coB6iz10gUsKqSHckRpu
=ScF1
-----END PGP SIGNATURE-----

--nextPart4646959.Ud0x7NujcZ--

Re: [patch, sim] Add isel instruction

[Joel Brobecker]

Abid,

> This patch adds support for isel instruction in the ppc simulator.
> Tested manually by running a small assembly file through the
> simulator. Is this ok?
> 
> Regards,
> Abid
> 
> sim/ChangeLog:
> 2013-04-19  Nathan Froyd  <froydnj@codesourcery.com>
> 
> 	* ppc-instructions (isel): New instruction.

This patch causes the sim build to fail when configured with
--enable-sim-float=spe. The full configure command-line used was:

    % configure --disable-nls --disable-tui --without-zlib --prefix=/[...]/install --enable-sim --enable-sim-float=spe --without-python --with-libexpat-prefix=/[...]/libexpat --target=powerpc-eabispe

The error I get is:

    | ./igen   -E  -F 32,f,o,e500 -CSRI 1024 -N 5  \
    | [...]
    |         expanding ...
    |         -n icache.h
    |         -h 
    |         -c tmp-icache.h
    | gen-icache.c:66: assertion failed - entry->nr_insn == 1
    | make: *** [tmp-igen] Error 1

I am not familiar with this part of the code, so can you please fix
or revert?

Interestingly, no problem with --enable-sim-float=altivec.

Thank you.

> 
> diff --git a/sim/ppc/ppc-instructions b/sim/ppc/ppc-instructions
> index 1b8fd89..d76a765 100644
> --- a/sim/ppc/ppc-instructions
> +++ b/sim/ppc/ppc-instructions
> @@ -3455,6 +3455,19 @@
> void::function::invalid_zero_divide_operation:cpu *processor,
> unsigned_word cia,
>  	PPC_INSN_MFCR(RT_BITMASK);
> 
>  #
> +# I.3.3.15 Fixed-Point Select
> +#
> +
> +0.31,6.RT,11.RA,16.RB,21.BF,26.15,31./:A::isel:Integer Select
> +	unsigned_word a;
> +	if (RA_is_0) a = 0;
> +	else         a = *rA;
> +	if (CR & (1 << (31 - BF)))
> +	  *rT = a;
> +	else
> +	  *rT = *rB;
> +
> +#

-- 
Joel

Re: [patch, sim] Add isel instruction

[Joel Brobecker]

> > sim/ChangeLog:
> > 2013-04-19  Nathan Froyd  <froydnj@codesourcery.com>
> > 
> > 	* ppc-instructions (isel): New instruction.

Another issue I noticed: The changeLog entry should be in sim/ppc/,
not sim/...

-- 
Joel

Re: [patch, sim] Add isel instruction

[Abid, Hafiz]

Joel,
Thansk for lettting me know. If this is urgent for you then I can  
revert as it may take me some time to find a fix.

Regards,
Abid

On 30/04/13 11:47:01, Joel Brobecker wrote:
> Abid,
> 
> > This patch adds support for isel instruction in the ppc simulator.
> > Tested manually by running a small assembly file through the
> > simulator. Is this ok?
> >
> > Regards,
> > Abid
> >
> > sim/ChangeLog:
> > 2013-04-19  Nathan Froyd  <froydnj@codesourcery.com>
> >
> > 	* ppc-instructions (isel): New instruction.
> 
> This patch causes the sim build to fail when configured with
> --enable-sim-float=spe. The full configure command-line used was:
> 
>     % configure --disable-nls --disable-tui --without-zlib  
> --prefix=/[...]/install --enable-sim --enable-sim-float=spe  
> --without-python --with-libexpat-prefix=/[...]/libexpat  
> --target=powerpc-eabispe
> 
> The error I get is:
> 
>     | ./igen   -E  -F 32,f,o,e500 -CSRI 1024 -N 5  \
>     | [...]
>     |         expanding ...
>     |         -n icache.h
>     |         -h
>     |         -c tmp-icache.h
>     | gen-icache.c:66: assertion failed - entry->nr_insn == 1
>     | make: *** [tmp-igen] Error 1
> 
> I am not familiar with this part of the code, so can you please fix
> or revert?
> 
> Interestingly, no problem with --enable-sim-float=altivec.
> 
> Thank you.
> 
> >
> > diff --git a/sim/ppc/ppc-instructions b/sim/ppc/ppc-instructions
> > index 1b8fd89..d76a765 100644
> > --- a/sim/ppc/ppc-instructions
> > +++ b/sim/ppc/ppc-instructions
> > @@ -3455,6 +3455,19 @@
> > void::function::invalid_zero_divide_operation:cpu *processor,
> > unsigned_word cia,
> >  	PPC_INSN_MFCR(RT_BITMASK);
> >
> >  #
> > +# I.3.3.15 Fixed-Point Select
> > +#
> > +
> > +0.31,6.RT,11.RA,16.RB,21.BF,26.15,31./:A::isel:Integer Select
> > +	unsigned_word a;
> > +	if (RA_is_0) a = 0;
> > +	else         a = *rA;
> > +	if (CR & (1 << (31 - BF)))
> > +	  *rT = a;
> > +	else
> > +	  *rT = *rB;
> > +
> > +#
> 
> --
> Joel
> 

Re: [patch, sim] Add isel instruction

[Joel Brobecker]

> Thansk for lettting me know. If this is urgent for you then I can
> revert as it may take me some time to find a fix.

I have reverted locally, so it's not urgent for me. But it isn't
nice to leave something deliberately broken for too long. Do you
have an idea of what "some time" would mean? I'll leave the decision
of whether and when to revert to Mike, who is the sim maintainer.

-- 
Joel

Re: [patch, sim] Add isel instruction

[Abid, Hafiz]

> Do you have an idea of what "some time" would mean?
By the end of this week.

On 30/04/13 12:05:14, Joel Brobecker wrote:
> > Thansk for lettting me know. If this is urgent for you then I can
> > revert as it may take me some time to find a fix.
> 
> I have reverted locally, so it's not urgent for me. But it isn't
> nice to leave something deliberately broken for too long. Do you
> have an idea of what "some time" would mean? I'll leave the decision
> of whether and when to revert to Mike, who is the sim maintainer.
> 
> --
> Joel
>