Refactor stringlike indexing primitives#2905
Merged
Conversation
to make it easier to add primitives for unboxed access without duplicating as much code Signed-off-by: David Vulakh <dvulakh@janestreet.com>
don't close over [mode] when constructing the [indexing_primitives] map, and hoist the computation out of [lookup_primitive] (previously we were recomputing the cartesian product each time we entered [lookup_primitive]) Signed-off-by: David Vulakh <dvulakh@janestreet.com>
mshinwell
reviewed
Aug 8, 2024
middle_end/flambda2/from_lambda/lambda_to_flambda_primitives.ml
Outdated
Show resolved
Hide resolved
mshinwell
reviewed
Aug 8, 2024
middle_end/flambda2/from_lambda/lambda_to_flambda_primitives.ml
Outdated
Show resolved
Hide resolved
Collaborator
|
Looks fine in outline, @TheNumbat please review in detail. |
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>
TheNumbat
requested changes
Aug 12, 2024
middle_end/flambda2/from_lambda/lambda_to_flambda_primitives.ml
Outdated
Show resolved
Hide resolved
middle_end/flambda2/from_lambda/lambda_to_flambda_primitives.ml
Outdated
Show resolved
Hide resolved
middle_end/flambda2/from_lambda/lambda_to_flambda_primitives.ml
Outdated
Show resolved
Hide resolved
middle_end/flambda2/from_lambda/lambda_to_flambda_primitives.ml
Outdated
Show resolved
Hide resolved
* change type of [bound_kind] in [bound] * move some type constraints to arguments * return old comment about bounds checks * move [is_substring] to [misc] Signed-off-by: David Vulakh <dvulakh@janestreet.com>
TheNumbat
approved these changes
Aug 12, 2024
Member
|
Looks good to merge once CI passes |
ITO444
pushed a commit
to ITO444/flambda-backend
that referenced
this pull request
Aug 13, 2024
* refactor string-like access primitives to make it easier to add primitives for unboxed access without duplicating as much code Signed-off-by: David Vulakh <dvulakh@janestreet.com> * hoist out [indexing_primitives] don't close over [mode] when constructing the [indexing_primitives] map, and hoist the computation out of [lookup_primitive] (previously we were recomputing the cartesian product each time we entered [lookup_primitive]) Signed-off-by: David Vulakh <dvulakh@janestreet.com> * make fmt Signed-off-by: David Vulakh <dvulakh@janestreet.com> * label [index] and [bound] in [check_bound] Signed-off-by: David Vulakh <dvulakh@janestreet.com> * move annotation in [actual_max_length_for_string] Signed-off-by: David Vulakh <dvulakh@janestreet.com> * move annotation in [string_like_load] Signed-off-by: David Vulakh <dvulakh@janestreet.com> * replace cartesian product helper with let operator Signed-off-by: David Vulakh <dvulakh@janestreet.com> * review changes - change type of [bound_kind] in [bound] - move some type constraints to arguments - return old comment about bounds checks - move [is_substring] to [misc] Signed-off-by: David Vulakh <dvulakh@janestreet.com> --------- Signed-off-by: David Vulakh <dvulakh@janestreet.com>
lukemaurer
pushed a commit
to lukemaurer/flambda-backend
that referenced
this pull request
Oct 23, 2024
* refactor string-like access primitives to make it easier to add primitives for unboxed access without duplicating as much code Signed-off-by: David Vulakh <dvulakh@janestreet.com> * hoist out [indexing_primitives] don't close over [mode] when constructing the [indexing_primitives] map, and hoist the computation out of [lookup_primitive] (previously we were recomputing the cartesian product each time we entered [lookup_primitive]) Signed-off-by: David Vulakh <dvulakh@janestreet.com> * make fmt Signed-off-by: David Vulakh <dvulakh@janestreet.com> * label [index] and [bound] in [check_bound] Signed-off-by: David Vulakh <dvulakh@janestreet.com> * move annotation in [actual_max_length_for_string] Signed-off-by: David Vulakh <dvulakh@janestreet.com> * move annotation in [string_like_load] Signed-off-by: David Vulakh <dvulakh@janestreet.com> * replace cartesian product helper with let operator Signed-off-by: David Vulakh <dvulakh@janestreet.com> * review changes - change type of [bound_kind] in [bound] - move some type constraints to arguments - return old comment about bounds checks - move [is_substring] to [misc] Signed-off-by: David Vulakh <dvulakh@janestreet.com> --------- Signed-off-by: David Vulakh <dvulakh@janestreet.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Refactor the logic for translating the
get/setprimitives forbigstring,string, andbytesin preparation for new versions of these primitives with unboxed indices and unboxed return types (#2906). As part of the refactor, string-like loads and stores now share bounds-checking logic with arrays.This PR does not add any new primitives. For existing primitives, I rely on existing tests.
I add a small test
ocaml/testsuite/tests/prim-bigstring/non_existent_primitives.mlto show that the as-yet unoccupied spaces in the (container type x index kind x data width) cartesian product continue to fail with sensible compiler errors.