Skip to content

Z: short circuit ArrayCmpLen if addresses are equal - #7222

Merged
hzongaro merged 1 commit into
eclipse-omr:masterfrom
ehsankianifar:ShortCircuitArrayCmpLen
Feb 1, 2024
Merged

Z: short circuit ArrayCmpLen if addresses are equal#7222
hzongaro merged 1 commit into
eclipse-omr:masterfrom
ehsankianifar:ShortCircuitArrayCmpLen

Conversation

@ehsankianifar

@ehsankianifar ehsankianifar commented Jan 10, 2024

Copy link
Copy Markdown
Contributor

If the addresses of two arrays are equal, those two arrays are considered equal and we do not need to actually compare those arrays. In this PR, we modified the instructions for x86 and there was a discussion to do the same for other architectures. On Z, it is more complicated and depending on the conditions and expected response, several methods implement array compare.
This PR targets array cmp len implementation. If the Array cmp asks for the position of the first mismatch, this implementation would be called. It returns the index of the fists mismatch or the length of the array if there is no mismatch.

Updated arraycmplen to short circuit if comparing the same array
Two arrays are equal if their addresses are equal
Added branch instruction to jump to return in this case

Signed-off-by: Ehsan Kiani Far <ehsan.kianifar@gmail.com>
@ehsankianifar

Copy link
Copy Markdown
Contributor Author

Tested for JDK8 (LoZ and zOS) Axxon 63769, test level.sanity, all tests pass
JDK11 (LoZ) Build #20122, test sanity.functional sanity.system sanity.openjdk, all tests pass.

@ehsankianifar

Copy link
Copy Markdown
Contributor Author

Instruction selection:

 [     0x2aa0bab1d00]	                       proc    	
 [     0x2aa0bab1e00]	                       fence   Relative [ 0x2aa0ba42580 ] BBStart <block_2> (frequency 10000)	
 [     0x2aa0bab2400]	                       LG      &GPR_0017, Parm[Parm  0<parm 0 L>] ?+0(GPR15)
 [     0x2aa0bad03b0]	                       LG      &GPR_0019, Parm[Parm  1<parm 1 L>] ?+8(GPR15)
 [     0x2aa0bad0670]	                       LG      GPR_0023, Parm[Parm  2<parm 2 J>] ?+16(GPR15)
 [     0x2aa0bad0d00]	                       LGR     GPR_0026,GPR_0023	
 [     0x2aa0bad0db0]	                       Label L0016:	# (Start of internal control flow)	
 [     0x2aa0bad0e70]	                       CLGR    &GPR_0017,&GPR_0019	
 [     0x2aa0bad0f20]	                       BRC     BH(0x8), Label L0017	
 [     0x2aa0bad0fe0]	                       LGR     GPR_0024,GPR_0023	
 [     0x2aa0bad1090]	                       LGR     GPR_0025,GPR_0023	
 [     0x2aa0bad1140]	                       CLCL    &GPR_0017,&GPR_0019	
 [     0x2aa0bad11f0]	                       SGR     GPR_0026,GPR_0024	
 [     0x2aa0bad1840]	                       assocreg
 [     0x2aa0bad12a0]	                       Label L0017:	# (End of internal control flow)	
 POST:
 {LegalEvenOfPair:&GPR_0017:R} {LegalOddOfPair:GPR_0024:R} {EvenOddPair:&GPR_0017:GPR_0024:R} {LegalEvenOfPair:&GPR_0019:R} {LegalOddOfPair:GPR_0025:R} {EvenOddPair:&GPR_0019:GPR_0025:R} {AssignAny:GPR_0023:R} {AssignAny:GPR_0026:R}
 [     0x2aa0bad1f50]	                       assocreg
 [     0x2aa0bad19a0]	                       retn    	
 POST:
 {GPR2:GPR_0026:R}
 [     0x2aa0bad2710]	                       fence   Relative [ 0x2aa0ba42584 ] BBEnd </block_2>	
 [     0x2aa0bad2cc0]	                       assocreg

dependencies->addPostCondition(orgLen, TR::RealRegister::AssignAny);
dependencies->addPostCondition(resultReg, TR::RealRegister::AssignAny);

generateRRInstruction(cg, TR::InstOpCode::getLoadRegOpCode(), node, resultReg, orgLen);

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

if the arrays are equal, we return the length in this method

cg->decReferenceCount(elemsExpr);
cg->decReferenceCount(firstBaseAddr);
cg->decReferenceCount(secondBaseAddr);
cursor->setDependencyConditions(dependencies);

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

dependency conditions added to the cFlowRegionEnd label

@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.

LGTM, thanks for the change.

@r30shah

r30shah commented Jan 12, 2024

Copy link
Copy Markdown
Contributor

Jenkins build zos,zlinux

1 similar comment
@r30shah

r30shah commented Jan 24, 2024

Copy link
Copy Markdown
Contributor

Jenkins build zos,zlinux

@r30shah

r30shah commented Jan 25, 2024

Copy link
Copy Markdown
Contributor

@hzongaro Can I request you to merge this change if you are OK with it ?

@hzongaro hzongaro self-assigned this Feb 1, 2024

@hzongaro hzongaro 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.

Looks good. Thanks!

@hzongaro

hzongaro commented Feb 1, 2024

Copy link
Copy Markdown
Contributor

x86-64 macOS failure is unrelated to this change. Merging.

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.

3 participants