Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Z: Fix arraycmplen opcode error #7140

Closed

Conversation

ehsankianifar
Copy link
Contributor

@ehsankianifar ehsankianifar commented Oct 10, 2023

The isArrayCmpSign function accept only arraycmp opcode.
Removed from arraycmplen since sign doesnt mean anything there

Fix: #7136
Signed-off-by: ehsan kiani far ehsan.kianifar@gmail.com

@ehsankianifar
Copy link
Contributor Author

isArrayCmpSign method has an assertion to make sure that the opcode is arraycmp but it was called in arraycmplen and was causing a failure. I just replaced that call with false since arraycmplen must return the index of the mismatch and sign flag doesn't mean anything there. sign flag is used to tell the compare helper to return -1, 0, or 1 in response.

@ehsankianifar
Copy link
Contributor Author

Testing:
used ArrayTest in compiler tril tests and manually changed the code to enable SIM path. I can confirm that it can use vectorized instructions.
gdb output:

=> 0x3fffc6000fa:	stg	%r15,120(%r15)
   0x3fffc600100:	lay	%r15,-160(%r15)
   0x3fffc600106:	stg	%r2,176(%r15)
   0x3fffc60010c:	stg	%r3,184(%r15)
   0x3fffc600112:	stg	%r4,192(%r15)
   0x3fffc600118:	lg	%r1,184(%r15)
   0x3fffc60011e:	lg	%r3,176(%r15)
   0x3fffc600124:	lg	%r0,192(%r15)
   0x3fffc60012a:	slgfi	%r0,1
   0x3fffc600130:	lgr	%r2,%r0
   0x3fffc600134:	vll	%v16,%r0,0(%r1)
   0x3fffc60013a:	vll	%v17,%r0,0(%r3)
   0x3fffc600140:	vfenebs	%v18,%v16,%v17
   0x3fffc600146:	jlh	0x3fffc600164
   0x3fffc60014a:	la	%r1,16(%r1)
   0x3fffc60014e:	la	%r3,16(%r3)
   0x3fffc600152:	slgfi	%r0,16
   0x3fffc600158:	jnle	0x3fffc600134
   0x3fffc60015c:	aghi	%r2,1
   0x3fffc600160:	j	0x3fffc600172

Copy link
Contributor

@r30shah r30shah left a comment

Choose a reason for hiding this comment

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

LGTM, @Spencer-Comin would appreciate, if you also review the change given that your recent work in this area.

@ehsankianifar , Can you rebase your change ?

@Spencer-Comin
Copy link
Contributor

LGTM

@r30shah
Copy link
Contributor

r30shah commented Oct 13, 2023

Jenkins build zos,zlinux

@r30shah
Copy link
Contributor

r30shah commented Oct 13, 2023

Just realized that couple of different commits are added in this PR.
@ehsankianifar After the above two build finishes, can you rebase your branch ?

The isArrayCmpSign function accept only arraycmp opcode
Removed from arraycmplen since sign doesnt mean anything there

Fix: eclipse#7136
Signed-off-by: ehsan kiani far ehsan.kianifar@gmail.com
@ehsankianifar
Copy link
Contributor Author

fixed in #7228

@ehsankianifar ehsankianifar deleted the FixArraycmplenOpcodeBugOnZ branch February 21, 2024 14:30
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.

Z arraycmplen tree evaluator throws error when try to use SIMD
3 participants