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.

Sign up for GitHub

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

Jump to bottom

Use 64 bits uniformly for length child of arraycmp opcode #6992

Closed
Spencer-Comin opened this issue May 17, 2023 · 4 comments · Fixed by #7313
Closed

Use 64 bits uniformly for length child of arraycmp opcode #6992

Spencer-Comin opened this issue May 17, 2023 · 4 comments · Fixed by #7313

Comments

@Spencer-Comin
Copy link
Contributor

During discussions related to #6951 it was mentioned that having the length input to arraycmp as a signed 32-bit integer is unnecessarily restrictive given that arrays can cover more than 231-1 bytes, and current platform implementations may not work as expected for lengths that exceed 231-1.
@Spencer-Comin Spencer-Comin mentioned this issue May 17, 2023
Closed
@Spencer-Comin
Copy link
Contributor Author

Also note that how the arraycmp length child is actually used is inconsistent with its definition, see #7011 (comment)

@Spencer-Comin Spencer-Comin changed the title 32 bits is too small for arraycmp length child Use 64 bits uniformly for length child of arraycmp opcode Feb 8, 2024
@ehsankianifar ehsankianifar mentioned this issue Feb 8, 2024
Merged
Spencer-Comin added a commit to Spencer-Comin/omr that referenced this issue Apr 16, 2024
@Spencer-Comin
Change arraycmp length child to 64 bits
0b9a87f 
Previously the arraycmp IL opcode's length child  was specified as 32 bits but
was inconsistent in its use. This commit changes the specification of the
length child to 64 bits, and normalizes the use to 64 bits.

Closes: eclipse#6992
Signed-off-by: Spencer Comin <spencer.comin@ibm.com>
@Spencer-Comin Spencer-Comin mentioned this issue Apr 16, 2024
Merged
Spencer-Comin added a commit to Spencer-Comin/omr that referenced this issue Apr 17, 2024
@Spencer-Comin
Change arraycmp length child to 64 bits
c758d04 
Previously the arraycmp IL opcode's length child  was specified as 32 bits but
was inconsistent in its use. This commit changes the specification of the
length child to 64 bits, and normalizes the use to 64 bits.

Closes: eclipse#6992
Signed-off-by: Spencer Comin <spencer.comin@ibm.com>
Spencer-Comin added a commit to Spencer-Comin/omr that referenced this issue Apr 17, 2024
@Spencer-Comin
Change arraycmp length child to 64 bits
b4cf92b 
Previously the arraycmp IL opcode's length child  was specified as 32 bits but
was inconsistent in its use. This commit changes the specification of the
length child to 64 bits, and normalizes the use to 64 bits.

Closes: eclipse#6992
Signed-off-by: Spencer Comin <spencer.comin@ibm.com>
@Spencer-Comin
Copy link
Contributor Author

@0xdaryl is this something that would warrant discussion at an architecture meeting?

Spencer-Comin added a commit to Spencer-Comin/omr that referenced this issue Apr 25, 2024
@Spencer-Comin
Change arraycmp length child to 64 bits
77b15ab 
Previously the arraycmp IL opcode's length child  was specified as 32 bits but
was inconsistent in its use. This commit changes the specification of the
length child to 64 bits, and normalizes the use to 64 bits.

Closes: eclipse#6992
Signed-off-by: Spencer Comin <spencer.comin@ibm.com>
Spencer-Comin added a commit to Spencer-Comin/omr that referenced this issue May 1, 2024
@Spencer-Comin
Change arraycmp length child to 64 bits
d756458 
Previously the arraycmp IL opcode's length child  was specified as 32 bits but
was inconsistent in its use. This commit changes the specification of the
length child to 64 bits, and normalizes the use to 64 bits.

Closes: eclipse#6992
Signed-off-by: Spencer Comin <spencer.comin@ibm.com>
Spencer-Comin added a commit to Spencer-Comin/omr that referenced this issue May 1, 2024
@Spencer-Comin
Change arraycmp length child to 64 bits
b159513 
Previously the arraycmp IL opcode's length child  was specified as 32 bits but
was inconsistent in its use. This commit changes the specification of the
length child to 64 bits, and normalizes the use to 64 bits.

Closes: eclipse#6992
Signed-off-by: Spencer Comin <spencer.comin@ibm.com>
Spencer-Comin added a commit to Spencer-Comin/omr that referenced this issue May 1, 2024
@Spencer-Comin
Change arraycmp length child to 64 bits
9589a57 
Previously the arraycmp IL opcode's length child  was specified as 32 bits but
was inconsistent in its use. This commit changes the specification of the
length child to 64 bits, and normalizes the use to 64 bits.

Closes: eclipse#6992
Signed-off-by: Spencer Comin <spencer.comin@ibm.com>
@hzongaro
Copy link
Member

Reopening as pull request #7313 needed to be reverted temporarily due to downstream breakage.

@hzongaro hzongaro reopened this Jun 14, 2024
@hzongaro
Copy link
Member

hzongaro commented Jul 9, 2024

Fixed by #7388

@hzongaro hzongaro closed this as completed Jul 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Change arraycmp length child to 64 bits Spencer-Comin/omr
2 participants
@hzongaro @Spencer-Comin