SDL-Hercules-390 / hyperion

The SDL Hercules 4.x Hyperion version of the System/370, ESA/390, and z/Architecture Emulator
Other
237 stars 89 forks source link

Initial Z/Vector E6 instructions #661

Closed JamesWekel closed 3 months ago

JamesWekel commented 3 months ago

An initial cut to introduce the Z/vector E6 instructions into the sharedvfp branch. Even though zvector2.c is still under development and needs significant changes to handle Decimal overflow, missing instructions, etc., I'm looking to merge changes into instruction handling infrastructure and avoid manual merge reconciliation to get back in sync with changes in the branch.

At this point, I'm my testing has been only on Linux.

VM/Community Edition 1.2 ipl'ed and I could logon to CMS, so I'm hoping that I got the overlapped instruction codes for Z/Vector E6 and ECPSVM E6 instructions.

ALL comments are welcome.

Jim

JamesWekel commented 3 months ago

Fish, not sure how I have one of your commits in my pull request.

Jim

JamesWekel commented 3 months ago

mcisho

Note a compile error with ieee.c:

ieee.c: In function ‘z900_vector_fp_load_lengthened’:
ieee.c:1519:42: error: ‘i’ undeclared (first use in this function)
 1519 |         GET_VECTOR_FLOAT64_OP(  op2, v2, i, regs );
[

and the windows build errors that Fish has identified in zvector-E6.zip

Fish,

  1. mcisho is working on ieee.c and zvector.c so I'll defer your comments to him.
  2. I was trying a Windows build using hercules-helper (while you were doing your review!) and noticed that zvector2 was missing so that was a short fix. Thanks for the intrinsic update. Until zvector2.c is finished, I'm not going to include any more intrinsic use.
  3. Since it doesn't build on windows, it's a bit hard to test!!

Thanks for the help.

Jim

mcisho commented 3 months ago

Aren't compilers a pita? Clang on Linux was quite happy the defines weren't there. I have done commits to resolve the ieee.c error and warnings (and many other changes that move the code forward). I haven't looked at the zvector.c warnings, they can wait awhile (imho!).

Fish-Git commented 3 months ago

Aren't compilers a pita?

Indubitably!

But this is of course part of the general PITA that portability itself entails. If we didn't care about operating system portability then things would be easy. We could target our product to just one operating system and one compiler. But as a consequence of our desired for our product to be portable across multiple operating systems and multiple compilers, such pita issues will always remain a part of our lives as Hercules developers. We may not like it, but unfortunately it is something we will have to continue to deal with as best we can.

JamesWekel commented 3 months ago

Fish,

With mcisho's commits to the sharedvfp branch, the commits & pull request now compile on both windows and Linux. Are there any additional changes that you recommend for this request?

Thanks, Jim

Fish-Git commented 3 months ago

With mcisho's commits to the sharedvfp branch, the commits & pull request now compile on both windows and Linux.

Not for me.  :(

I pulled your latest commits to your fork:

and it still fails to build on both Windows and Linux.

It looks like you only made some, but not ALL, of the requested changes that I listed earlier, but the other error/warnings still remain. The end result is, it unfortunately STILL does not build successfully, neither on Windows nor on Linux:

In file included from ./ieee.c:6991:
./ieee.c:1519:42: error: use of undeclared identifier 'i'
./ieee.c:1670:21: warning: unused variable 'op1' [-Wunused-variable]
./ieee.c:1684:21: warning: unused variable 'op1' [-Wunused-variable]
./ieee.c:1697:21: warning: unused variable 'op1' [-Wunused-variable]
./ieee.c:1733:21: warning: unused variable 'op1' [-Wunused-variable]
./ieee.c:1747:21: warning: unused variable 'op1' [-Wunused-variable]
./ieee.c:1760:21: warning: unused variable 'op1' [-Wunused-variable]
./ieee.c:1796:21: warning: unused variable 'op1' [-Wunused-variable]
./ieee.c:1810:21: warning: unused variable 'op1' [-Wunused-variable]
./ieee.c:1823:21: warning: unused variable 'op1' [-Wunused-variable]
./ieee.c:1859:21: warning: unused variable 'op1' [-Wunused-variable]
./ieee.c:1873:21: warning: unused variable 'op1' [-Wunused-variable]
./ieee.c:1886:21: warning: unused variable 'op1' [-Wunused-variable]
./ieee.c:1922:21: warning: unused variable 'op1' [-Wunused-variable]
./ieee.c:1936:21: warning: unused variable 'op1' [-Wunused-variable]
./ieee.c:1949:21: warning: unused variable 'op1' [-Wunused-variable]
./ieee.c:1985:21: warning: unused variable 'op1' [-Wunused-variable]
./ieee.c:1999:21: warning: unused variable 'op1' [-Wunused-variable]
./ieee.c:2012:21: warning: unused variable 'op1' [-Wunused-variable]
./ieee.c:2048:21: warning: unused variable 'op1' [-Wunused-variable]
./ieee.c:2062:21: warning: unused variable 'op1' [-Wunused-variable]
./ieee.c:2075:21: warning: unused variable 'op1' [-Wunused-variable]
./ieee.c:2117:21: warning: unused variable 'op1' [-Wunused-variable]
./ieee.c:2130:21: warning: unused variable 'op1' [-Wunused-variable]
./ieee.c:2143:21: warning: unused variable 'op1' [-Wunused-variable]
./ieee.c:2182:21: warning: unused variable 'op1' [-Wunused-variable]
./ieee.c:2195:21: warning: unused variable 'op1' [-Wunused-variable]
./ieee.c:2208:21: warning: unused variable 'op1' [-Wunused-variable]
27 warnings and 1 error generated.
make[2]: *** [Makefile:2637: ieee.lo] Error 1
make[2]: *** Waiting for unfinished jobs....
make[1]: *** [Makefile:2687: all-recursive] Error 1
make: *** [Makefile:2006: all] Error 2

I'm afraid I still cannot approve your Pull Request as it currently stands. Sorry!  :(

Are there any additional changes that you recommend for this request?

Yes!   See above.

Fish-Git commented 3 months ago

To be clear, Hercules's sharedvfp branch does indeed build cleanly on both Windows and Linux.

The zvector-E6 branch of YOUR repository (fork) however -- the branch of your fork with the changes you want me to merge (pull) into Hercules -- still does not.

JamesWekel commented 3 months ago

Fish, I've submitted a revised pull request. I'm closing this request.

Jim

Fish-Git commented 3 months ago

Fish, I've submitted a revised pull request. I'm closing this request.

10-4. I'll take a look at you new request.