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

Support for constant arrays of numeric element types. #2021

Merged
merged 19 commits into from
Sep 27, 2023
Merged

Conversation

fridis
Copy link
Member

@fridis fridis commented Sep 25, 2023

This reduces the size of the generated code, in particular for the JVM backend that previously was not able to create bytecode for the tables used for ryu float to string conversion code.

Now, constant arrays are serialized into an array of bytes that is then processed by the backend to produce code that creates the array.

@fridis fridis mentioned this pull request Sep 25, 2023
@fridis fridis marked this pull request as ready for review September 26, 2023 08:22
src/dev/flang/air/Clazzes.java Outdated Show resolved Hide resolved
src/dev/flang/me/MiddleEnd.java Outdated Show resolved Hide resolved
src/dev/flang/fuir/FUIR.java Show resolved Hide resolved
src/dev/flang/fuir/analysis/dfa/DFA.java Outdated Show resolved Hide resolved
src/dev/flang/be/jvm/JVM.java Show resolved Hide resolved
src/dev/flang/be/jvm/JVM.java Outdated Show resolved Hide resolved
michaellilltokiwa and others added 18 commits September 26, 2023 14:56
…he heap.

The problem was that a call of the form

  f =>
    [1,2,3].as_list

will pass a ref to the array to `array.as_list`, which will create a Cons cell
that contains that ref. Since that Cons cell with be returned as the list result
of `f`, we have a dangling reference.

Before array consts, this was solved since the array was instanciated via a call
and the call result was heap cloned explicitly.

In a later step, I would like to see all const arrays stored globally and
allocated only once such that all the cloning would no longer be a problem.

As for heap cloning for call results in the C backend: This is done way too, the
plan is to find a common solution for this in the JVM and C backend, so no need
to worry right now.
…ay elements

The problem was that the comparison of `SysArrays` did not take the element
values into account, such that joining the `SysArray`s of `[123]` with `[]`
could result in the `SysArray` for `[]`, whose element values was `null`, i.e.,
`void` resulting in DFA to stop analysis when accessing the elements of `[123]`.

This patch fixes `SysArray.compareTo` to compare `_elements`. Additionally, it
adds some infrastucture that may help in the future to extract the element
values from the data array of a constant array.

This patch also add element values for the result of `fuzion.sys.fileio.mmap`
which was missing and worked only by chance.
…, length

The original code using `FUIR.clazzField` with constant indices is very brittle
because it is sensitive to changes in the order of declared fields.
Constants of non-primitive types, i.e., arrays and constant strings, are now
created globally in fzC_universe during class initialization.

Duplicate constants are detected and removed.
…tant arrays

Replaced `forEach` by explicit lookups.
…Java_Ref

This got lost somewhere in the pulls and merges.
@fridis
Copy link
Member Author

fridis commented Sep 26, 2023

I have added commits for the requested changes plus two additional commits:

This is required, e.g., for constants `i16 0` that are no longer a call to
`i16`, but constants now.
@maxteufel
Copy link
Collaborator

I intentionally removed these during the merge due to your comment saying to remove them as they're probably not necessary.

@fridis
Copy link
Member Author

fridis commented Sep 27, 2023

I intentionally removed these during the merge due to your comment saying to remove them as they're probably not necessary.

Do not trust me!

@maxteufel
Copy link
Collaborator

I intentionally removed these during the merge due to your comment saying to remove them as they're probably not necessary.

Do not trust me!

I have tested both variants before making the pull request, which is why I asked you here.

@maxteufel
Copy link
Collaborator

I actually just noticed a lot more JVM tests were failing with messages referencing fuzion.java.Java_Object, and this does seem to be related to markUsed, so we do actually need that.

With markUsed calls in MiddleEnd 115/144 tests passed, 8 skipped, 21 failed., without: 42/144 tests passed, 8 skipped, 94 failed.

@fridis fridis merged commit 33e7e2d into main Sep 27, 2023
5 checks passed
@maxteufel maxteufel deleted the const_arrays branch September 27, 2023 07:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants