SDL-Hercules-390 / hyperion

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

ICOUNT changes to accomodate E7xx family, PRINT_INST & qsort #621

Closed salva-rczero closed 7 months ago

salva-rczero commented 7 months ago

In preparation for possible changes that may become necessary for the vector facility (129) and as a practical exercise of how pulls requests work in this project, I request these changes in the behavior of icount:

1.- Counters & times tables in hstructs.h & opcode.h for E7xx instruction family.

2.- Add PRINT_INST to ouput line

hscemode.c(2865)  HHC02292I Sorted icount display: 
hscemode.c(2912)  HHC02292I Inst 'E371' count            2 (28%) time          617 (308.500000) LAY   0,0(0,0)               load_address_y  
hscemode.c(2912)  HHC02292I Inst 'A704' count            1 (14%) time            5 (  5.000000) BRC   0,*+0                  branch_relative_on_condition                   
hscemode.c(2912)  HHC02292I Inst '05'   count            1 (14%) time          724 (724.000000) BALR  0,0                    branch_and_link_register                       
hscemode.c(2912)  HHC02292I Inst 'E706' count            1 (14%) time            0 (  0.000000) ????? ,                      ?                                              
hscemode.c(2912)  HHC02292I Inst 'EB2F' count            1 (14%) time          347 (347.000000) LCTLG 0,0,0(0)               load_control_long                              
hscemode.c(2912)  HHC02292I Inst '00'   count            1 (14%) time            0 (  0.000000) ????? ,                      ?                                              
cmdtab.c(455)     HHC90000D DBG: RC = 0                                                               

3.- Change arrays to struct ICOUNT_INSTR and iterative sort to a cleanest qsort.

p.d. This is my first pull request. Please, be patient.

Fish-Git commented 7 months ago

Merged!

salva-rczero commented 7 months ago

Thank you so much @Fish-Git.

If I don't abuse your patience, I would like to ask you about two other changes that I was not sure whether to make or not:

At opcode.c "used" var is used to show "HHC02292I First use" message, but imapxx arrays are U64. This causes the var to overflow on long tests for LOAD, BCTR and other commonly used instructions and the "Fisrt use" message to appear more than once. Is it delivered or should it be changed to U64?

In my first tests I forget to add imape7 & imape7T at hstructs.h in the definition of IMAP_SIZE:

#define IMAP_SIZE \
            ( sizeof(sysblk.imap01) \
            + sizeof(sysblk.imapa4) \
...

Wouldn't it be cleaner to define all "imapxx" inside a sub-Struct? Or is there any reason (performance, alignment...) not to use sub-Structs?

Regards, salva.

Fish-Git commented 7 months ago

At opcode.c "used" var is used to show "HHC02292I First use" message, but imapxx arrays are U64. This causes the var to overflow on long tests for LOAD, BCTR and other commonly used instructions and the "Fisrt use" message to appear more than once. Is it delivered or should it be changed to U64?

You mean in the "BEG_COUNT_INSTR" macro in opcode.h? Yes. You are correct. It should be U64, and I have made the fix. Thank you!

Wouldn't it be cleaner to define all "imapxx" inside a sub-Struct? Or is there any reason (performance, alignment...) not to use sub-Structs?

I don't understand what you mean. What is a sub-struct?

salva-rczero commented 7 months ago

Thank you @Fish-Git.

I mean a Struct inside Struct. I'll provide a tentative pull request for your consideration.