Skip to content

Kill correct registers for VLM - #6748

Merged
0xdaryl merged 1 commit into
eclipse-omr:masterfrom
Spencer-Comin:fix-vlm-ra
Oct 27, 2022
Merged

Kill correct registers for VLM#6748
0xdaryl merged 1 commit into
eclipse-omr:masterfrom
Spencer-Comin:fix-vlm-ra

Conversation

@Spencer-Comin

Copy link
Copy Markdown
Contributor

Account for vector registers when calculating real register to check if spill is needed after a load multiple instruction. This fixes the behaviour of uneccessarily and incorrectly spilling GPRs when using VLM. An example of the error from my experience trying to accelerate crc32c on Z:

[     0x3ff8dcbd580]                          VLM     VRF_0095,VRF_0098,#415 0(&GPR_0033)VRF_0095(0/6)~FPR5 VRF_0098(0/6)~FPR8 
details:                      trying to free GPR_0099 for killed reg GPR6 by loadmultiple 

details:                      BEST FREE REG for GPR_0099 is GPR14 

 [     0x3ff8ddb3610]                          LGR     GPR6,GPR14       details:                      trying to free GPR_0050 for killed reg GPR7 by loadmultiple 

details:                      BEST FREE REG for GPR_0050 is GPR9

 [     0x3ff8ddb37f0]                          LGR     GPR7,GPR9      
 [     0x3ff8dcbd580]                          VLM     VRF5,VRF8,#415 0(GPR2)

Here GPR7 and GPR6 are unnecessarily spilled since GPR5-GPR8 are incorrectly used in the check at [1] rather than VRF5-VRF8. This patch solves this problem.

[1] https://github.com/eclipse/omr/blob/f99c9dd2248e6ee35e40536b3c243eee33924407/compiler/z/codegen/OMRInstruction.cpp#L722

@Spencer-Comin

Copy link
Copy Markdown
Contributor Author

@r30shah @joransiu

@r30shah

r30shah commented Oct 19, 2022

Copy link
Copy Markdown
Contributor

Jenkins build zos,zlinux

@joransiu joransiu left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@r30shah r30shah left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change Looks Good to Me.
@0xdaryl As z/OS and Linux on Z builds have passed and Spencer has done testing through personal builds as well for this change, Can I request you to merge this change?

Account for vector registers when calculating real register to check if spill is needed after a load multiple instruction.

Signed-off-by: Spencer Comin <spencer.comin@ibm.com>
@r30shah

r30shah commented Oct 21, 2022

Copy link
Copy Markdown
Contributor

Relaunching build with the rebased changes to make sure nothing breaks

@r30shah

r30shah commented Oct 21, 2022

Copy link
Copy Markdown
Contributor

Jenkins build zos,zlinux

@r30shah

r30shah commented Oct 24, 2022

Copy link
Copy Markdown
Contributor

@0xdaryl Can you please review/merge this change?

@0xdaryl 0xdaryl self-assigned this Oct 27, 2022
@0xdaryl
0xdaryl merged commit 7bb81bf into eclipse-omr:master Oct 27, 2022
@Spencer-Comin
Spencer-Comin deleted the fix-vlm-ra branch October 27, 2022 14:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants