-
Notifications
You must be signed in to change notification settings - Fork 57
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
Improve specialization optimization #2944
Merged
Merged
Changes from all commits
Commits
Show all changes
12 commits
Select commit
Hold shift + click to select a range
be5e07a
save pragma info for letrec and lambda
lukaszcz f7cfe41
fix names in JuvixTree
lukaszcz 941814c
remove assumption that type variables are at the front
lukaszcz 3b03eb2
bugfix
lukaszcz 2937eac
allow specialization of constructor creation
lukaszcz afa4c87
make name disambiguation rename specialization pragma args
lukaszcz 327098c
tests
lukaszcz 9a95510
ormolu
lukaszcz 7303a62
pragmas in record definitions
lukaszcz 3372638
ranges
lukaszcz ae7c1eb
fix tests
lukaszcz e396a01
test
lukaszcz File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
Submodule juvix-stdlib
updated
12 files
+2 −0 | Stdlib/Data/Bool.juvix | |
+4 −0 | Stdlib/Data/List.juvix | |
+4 −0 | Stdlib/Data/Maybe.juvix | |
+2 −0 | Stdlib/Data/Pair.juvix | |
+54 −25 | Stdlib/Data/Range.juvix | |
+4 −0 | Stdlib/Data/Result.juvix | |
+3 −0 | Stdlib/Data/Unit.juvix | |
+1 −0 | Stdlib/Prelude.juvix | |
+4 −2 | Stdlib/Trait/Foldable/Monomorphic.juvix | |
+2 −2 | Stdlib/Trait/Foldable/Polymorphic.juvix | |
+5 −0 | Stdlib/Trait/Functor/Monomorphic.juvix | |
+5 −0 | Stdlib/Trait/Functor/Polymorphic.juvix |
This file contains 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
This file contains 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
This file contains 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
This file contains 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
This file contains 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
This file contains 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
This file contains 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
This file contains 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
This file contains 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
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -19,7 +19,10 @@ isSpecializable md node = | |
_ -> True | ||
NLam {} -> True | ||
NCst {} -> True | ||
NCtr Constr {..} -> all (isSpecializable md) _constrArgs | ||
NCtr Constr {..} -> | ||
case lookupConstructorInfo md _constrTag ^. constructorPragmas . pragmasSpecialise of | ||
Just (PragmaSpecialise False) -> False | ||
_ -> True | ||
NApp {} -> | ||
let (h, _) = unfoldApps' node | ||
in isSpecializable md h | ||
|
@@ -102,7 +105,11 @@ convertNode = dmapLRM go | |
(tyargs, tgt) = unfoldPi' (ii ^. identifierType) | ||
def = lookupIdentifierNode md _identSymbol | ||
(lams, body) = unfoldLambdas def | ||
argnames = map (^. lambdaLhsBinder . binderName) lams | ||
argnames = | ||
zipWith | ||
(\mn lhs -> fromMaybe (lhs ^. lambdaLhsBinder . binderName) mn) | ||
(ii ^. identifierArgNames ++ repeat Nothing) | ||
lams | ||
|
||
-- arguments marked for specialisation with `specialize: true` | ||
psargs0 = | ||
|
@@ -118,23 +125,21 @@ convertNode = dmapLRM go | |
| (isJust pspec || isJust pspecby || not (null psargs0)) && length args == argsNum -> do | ||
let psargs1 = mapMaybe getArgIndex $ maybe [] (^. pragmaSpecialiseArgs) pspec | ||
psargs2 = maybe [] (map (+ 1) . mapMaybe (`elemIndex` argnames) . (^. pragmaSpecialiseBy)) pspecby | ||
-- psargs are the arguments explicitly marked for specialization | ||
psargs = nubSort (psargs0 ++ psargs1 ++ psargs2) | ||
-- assumption: all type variables are at the front | ||
let specargs0 = | ||
let -- specargs0 are the arguments actually selected for specialization | ||
specargs0 = | ||
filter | ||
( \argNum -> | ||
argNum <= argsNum | ||
&& isSpecializable md (args' !! (argNum - 1)) | ||
&& isArgSpecializable md _identSymbol argNum | ||
) | ||
psargs | ||
tyargsNum = length (takeWhile (isTypeConstr md) tyargs) | ||
tyargnums = map fst $ filter (isTypeConstr md . snd) $ zip [1 .. argsNum] tyargs | ||
-- in addition to the arguments explicitly marked for | ||
-- specialisation, also specialise all type arguments | ||
specargs = | ||
nub $ | ||
[1 .. tyargsNum] | ||
++ specargs0 | ||
specargs = nub $ tyargnums ++ specargs0 | ||
-- the arguments marked for specialisation which we don't | ||
-- specialise now | ||
remainingSpecargs = | ||
|
@@ -165,12 +170,9 @@ convertNode = dmapLRM go | |
| null specargs0 -> | ||
return $ End (mkApps' (NIdt idt) args') | ||
| otherwise -> do | ||
eassert (tyargsNum < argsNum) | ||
eassert (length lams == argsNum) | ||
eassert (length args' == argsNum) | ||
eassert (argsNum <= length tyargs) | ||
-- assumption: all type variables are at the front | ||
eassert (not $ any (isTypeConstr md) (drop tyargsNum tyargs)) | ||
massert (length lams == argsNum) | ||
massert (length args' == argsNum) | ||
massert (argsNum <= length tyargs) | ||
-- the specialisation signature: the values we specialise the arguments by | ||
let specSigArgs = selectSpecargs specargs args' | ||
specSig = (specSigArgs, specargs) | ||
|
@@ -237,8 +239,8 @@ convertNode = dmapLRM go | |
| otherwise -> | ||
return $ End $ mkApps' (NIdt idt) args' | ||
|
||
-- assumption: all type arguments are substituted, so no binders in the type | ||
-- list refer to other elements in the list | ||
-- Because all type arguments are substituted (specialized), in the end no | ||
-- binders in the resulting type list refer to other elements in the list | ||
removeSpecTypeArgs :: [Int] -> [Node] -> [Type] -> [Type] | ||
removeSpecTypeArgs = goRemove 1 | ||
where | ||
|
@@ -269,7 +271,7 @@ convertNode = dmapLRM go | |
|
||
shiftSpecargs :: [Int] -> [Int] -> [Int] | ||
shiftSpecargs specargs = | ||
map (\argNum -> argNum - length (filter (argNum <) specargs)) | ||
map (\argNum -> argNum - length (filter (argNum >) specargs)) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Does the change from There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That was incorrect, but we didn't see it because:
|
||
|
||
-- Replace the calls to the function being specialised with the specialised | ||
-- version (omitting the specialised arguments). We need to first replace | ||
|
This file contains 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
This file contains 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
This file contains 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
Oops, something went wrong.
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why has this changed from 4 to 6?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because with maps and folds inside record definitions 4 iterations are not enough to inline & specialize everything in a typical case (without further nestings).