Skip to content

Unboxed load/store primitives for string-likes#2906

Merged
dvulakh merged 8 commits intomainfrom
dvulakh.stringlike-unboxed-access-primitives
Aug 15, 2024
Merged

Unboxed load/store primitives for string-likes#2906
dvulakh merged 8 commits intomainfrom
dvulakh.stringlike-unboxed-access-primitives

Conversation

@dvulakh
Copy link
Copy Markdown
Contributor

@dvulakh dvulakh commented Aug 8, 2024

Add primitives for:

  • Load/store
  • From: bigstring, string, and bytes
  • Index by: int, int32#, int64#, nativeint#
  • Data width: int16, int32, float32, int64, int128 (both boxed and unboxed when possible)

I add tests of operation over int128 to the simd/arrays.ml test. tests/typing-layouts/generate_stringlike_indexing.ml generates a test (tests/typing-layouts/unboxed_int_stringlike_indexing.ml) that stresses the remaining primitives.

@dvulakh dvulakh marked this pull request as draft August 8, 2024 15:31
@dvulakh dvulakh force-pushed the dvulakh.stringlike-unboxed-access-primitives branch from 263f553 to 6f3ebe0 Compare August 8, 2024 15:44
@mshinwell
Copy link
Copy Markdown
Collaborator

Looks ok in outline, @TheNumbat will review in detail.

@mshinwell mshinwell added flambda2 Prerequisite for, or part of, flambda2 runtime labels Aug 8, 2024
@dvulakh dvulakh force-pushed the dvulakh.stringlike-unboxed-access-primitives branch 2 times, most recently from e26aec5 to e3ec9b4 Compare August 12, 2024 18:50
Base automatically changed from dvulakh.prepare-for-unboxed-indexing-primitives to main August 12, 2024 21:13
@dvulakh dvulakh force-pushed the dvulakh.stringlike-unboxed-access-primitives branch from e3ec9b4 to a98df86 Compare August 14, 2024 17:21
index by:     int32#,int64#,nativeint#
access width: int16,int32,float32,int64,int128,int32#,float32#,int64#

for bigstring, string, bytes

Signed-off-by: David Vulakh <dvulakh@janestreet.com>
Signed-off-by: David Vulakh <dvulakh@janestreet.com>
Signed-off-by: David Vulakh <dvulakh@janestreet.com>
Signed-off-by: David Vulakh <dvulakh@janestreet.com>
Signed-off-by: David Vulakh <dvulakh@janestreet.com>
Signed-off-by: David Vulakh <dvulakh@janestreet.com>
Signed-off-by: David Vulakh <dvulakh@janestreet.com>
@dvulakh dvulakh force-pushed the dvulakh.stringlike-unboxed-access-primitives branch from 9cc1453 to 5a79ac1 Compare August 14, 2024 22:36
@dvulakh dvulakh marked this pull request as ready for review August 14, 2024 22:40
@dvulakh dvulakh requested a review from TheNumbat August 14, 2024 22:40
fixes ci

Signed-off-by: David Vulakh <dvulakh@janestreet.com>
@dvulakh dvulakh marked this pull request as draft August 15, 2024 18:23
@dvulakh
Copy link
Copy Markdown
Contributor Author

dvulakh commented Aug 15, 2024

I noticed a problem with the code generated by these primitives. For example, %caml_string_get64u#_indexed_by_int64# generates:

salq $1, %rbx
sarq $1, %rbx
movq (%rax,%rbx), %rax
ret

This is because the flambda primitives operate on tagged indices, and the untagging doesn't happen until conversion to cmm. I'll post a fix in another PR (edit: #2930); meanwhile I'm reopening review here.

@dvulakh dvulakh marked this pull request as ready for review August 15, 2024 19:09
Copy link
Copy Markdown
Member

@TheNumbat TheNumbat left a comment

Choose a reason for hiding this comment

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

LGTM, adding the primitives is mostly mechanical after the refactoring, and the tests are quite thorough. I will review the improved codegen PR as well.

@dvulakh dvulakh merged commit 3420347 into main Aug 15, 2024
@dvulakh dvulakh deleted the dvulakh.stringlike-unboxed-access-primitives branch August 15, 2024 20:19
lukemaurer pushed a commit that referenced this pull request Oct 23, 2024
* unboxed access primitives for string-likes

index by:     int32#,int64#,nativeint#
access width: int16,int32,float32,int64,int128,int32#,float32#,int64#

for bigstring, string, bytes

Signed-off-by: David Vulakh <dvulakh@janestreet.com>

* make fmt

Signed-off-by: David Vulakh <dvulakh@janestreet.com>

* remove [CAMLprim] from macro names

Signed-off-by: David Vulakh <dvulakh@janestreet.com>

* tweak formatting in [lambda.ml{,i}]

Signed-off-by: David Vulakh <dvulakh@janestreet.com>

* improve generated tests

Signed-off-by: David Vulakh <dvulakh@janestreet.com>

* better data in generated tests

Signed-off-by: David Vulakh <dvulakh@janestreet.com>

* add more tests of edge-case indices

Signed-off-by: David Vulakh <dvulakh@janestreet.com>

* reflect float32 -> stable in tests

fixes ci

Signed-off-by: David Vulakh <dvulakh@janestreet.com>

---------

Signed-off-by: David Vulakh <dvulakh@janestreet.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

flambda2 Prerequisite for, or part of, flambda2 runtime

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants