Restored array_to_subslice3 & code quality optimizations following the DST PR

[Lin23299]

According to #312 , I first tried to restore this optimization using array_to_subslice3. I think the result is ok but what's happening here feels strange for me.

At this phase (PreCleanup3), we are rewriting array_to_subslice using Range into a array_to_subslice3 using 2 size_ts , but the difference here is that we have the monomorphized definition of Eurydice_array_to_subslice_XX. We have to remove it after this optimization. So basically we define the function array_to_subslice using abstract syntax with Dst_Ref, and replace it later with a C macro by passing the monomorphized return type Dst_Ref_XX.

Back to the quality issue, it seems to me that the point is to get rid of the Range type. If so, maybe a better solution is to rewrite them earlier before the monomorphization, therefore we can get a monomorphized func def of array_to_subslice3_XX using size_ts instead of C macro definition?

For the general optimization point, I agree that we could have many improvements. Following the discussion above, I think the question is to choose from:

1. builtin functions + their monomorphized implementations in the generated C file (the current version)
2. builtin functions + C macro definitions (the version before DST PR)
3. directly rewriting these functions using abstract syntax (e.g. slice_index(s,n,t) -> s.ptr[n])

I'm not sure which one is better, my previous plan is based on 1. Actually the current version is a mixup of 1 and 2 cuz I only wrote the abstract syntax for the functions could not be defined using macro. The array_to_subslice3 issue implies that it is possible to switch from 1 back to 2 for these functions. @protz

Edit: I just found that the current macro definition causes stack smashing in test-libcrux and I don't know why. This is a reason I personally prefer 1 than 2 because C macro is more error-prone.

[protz]

You are correct. Having these as a late-phase cleanup that is untyped was necessary only because the definition of slice was opaque, and now this is not a great solution.

I think 2. should be definitely ruled out, for the reasons that you mention.

I think we should do a mix of 1. and 3. I think it's mostly going to boil down to a cosmetic / taste factor, as to what we think is most readable in the C code:

• slice_len(e) ~~> e.meta is definitely a case of 3., but can be done as an earlier phase that is well-typed; same deal with slice_index, etc.
• array_to_subslice with a constant Range argument can become a call to array_to_subslice_range (let's pick a more readable name than array_to_subslice_3!), which perhaps is more of a case of 1... or do you think that (Eurydice_slice_XX)({ .ptr = arr + start, .len = end - start }) is more readable?

An additional complication is that fn<const K>(x: [T; N]) { x.to_subslice(0, K) } will be have constant arguments only after monomorphization, so perhaps we will miss some opportunities to simplify calls to array_to_sublice

What do you think?

[protz]

Looking at the diff I agree with you that we want to avoid macros as much as possible, and reverting to the old style where the macro receives a type argument is definitely a regression, so let's not do that.

[Lin23299]

You are correct. Having these as a late-phase cleanup that is untyped was necessary only because the definition of slice was
I think 2. should be definitely ruled out, for the reasons that you mention.

I think we should do a mix of 1. and 3. I think it's mostly going to boil down to a cosmetic / taste factor, as to what we think is most readable in the C code:

• slice_len(e) ~~> e.meta is definitely a case of 3., but can be done as an earlier phase that is well-typed; same deal with slice_index, etc.
• array_to_subslice with a constant Range argument can become a call to array_to_subslice_range (let's pick a more readable name than array_to_subslice_3!), which perhaps is more of a case of 1... or do you think that (Eurydice_slice_XX)({ .ptr = arr + start, .len = end - start }) is more readable?

An additional complication is that fn<const K>(x: [T; N]) { x.to_subslice(0, K) } will be have constant arguments only after monomorphization, so perhaps we will miss some opportunities to simplify calls to array_to_sublice

Totally agreed. I'll try to implement these optimizations after helping @ssyram to fix the issues on #311