Draft pull request for all changes for ECOOP 26 submission #31
Conversation
ulysses4ever
left a comment
ulysses4ever
left a comment
There was a problem hiding this comment.
CI has to be fixed but otherwise it's a good progress, thanks a lot!
|
|
||
| {-# INLINE free #-} | ||
| free :: HasPrim a => Array a -. () | ||
| free :: Array a -. () |
There was a problem hiding this comment.
I'm a little surpriced you don't need the HasPrim context for all configurations to typecheck.
| -. Array srcs -. Array dsts | ||
| allocScratch i a f arr = | ||
| let | ||
| !(dst, tmp) = f arr (makeArray i a) |
There was a problem hiding this comment.
alloc above has two implementations that use different versions of make (filling and non-filling, low-level one). Probably, this place should do the same. But duplicationg CPP doesn't look nice. I'd rather we have two implementations of one make function, which we can call anywhere (inside TCB, of course) and not think about the different implementations.
src/DpsMergeSort.hs
Outdated
There was a problem hiding this comment.
Maybe worth renaming the 4-version into this. I always felt like 4 looks weird. Although, it may feel natural for algorithms people. But we're not submitting to an algorithms venue (at the moment).
| let (Ur n, xs1) = A.size2 xs in | ||
| if n == 0 then xs1 | ||
| else quickSortBtw 0 n xs1 |
There was a problem hiding this comment.
the formatting is a little unconventional :-) but I'm not too stressed about it in functions as little as this one.
| then truncate((18820.2738 / (exp ((log (fromIntegral len)) * 0.5) )) :: Float) | ||
| else 28) src' (A.makeArray len anyVal) in | ||
| case A.free _tmp of !() -> src'' | ||
| let !(Ur len, src') = A.size2 src -- below expression is always in the interval [28, 708] (interval changed from meeting doc). |
There was a problem hiding this comment.
Note for future: these horrible constants should be acknowledged somewhere.
| isort_tmp :: A.Array a -. A.Array a -. (A.Array a, A.Array a) | ||
| isort_tmp src tmp = | ||
| let !(old_arr, tmp_arr) = A.copy2 0 0 n src tmp | ||
| in (isort 0 (tmp_arr ? lem_equal_slice_bag src tmp 0 n), old_arr) | ||
| ? lem_copy_equal_slice src 0 tmp 0 n -- there's an issue with using !() |
There was a problem hiding this comment.
I'm generally not a big fan of leaving behind screenfulls of commented code. But I'm not dying on this hill!
Draft, do not merge yet