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

cps2ds - wrong body type #222

Open
leissa opened this issue May 22, 2023 · 4 comments
Open

cps2ds - wrong body type #222

leissa opened this issue May 22, 2023 · 4 comments

Comments

@leissa
Copy link
Member

leissa commented May 22, 2023

<unknown location>: error: body 'cps_call_55166' of lambda is of type 
'⊥:(★)' but its codomain is of type 
'.Idx 2'

Steps to reproduce:

git pull
git checkout bug/cps2ds_bug
make 
cd lit
./probe.sh direct/cps_bug.thorin
@NeuralCoder3
Copy link
Collaborator

make

A makefile might be a good idea to simplify using the cmake commands. (Especially since the lit commands as described in the documentation do not work me right now).

@NeuralCoder3
Copy link
Collaborator

NeuralCoder3 commented May 23, 2023

I think I identified the problem:

The code produced after ds2cps (before the failing cps2ds pass) is:

.lam Uf_54920 _54941: «2; %math.F (52, 11)»: ★ = {
    .Idx 2
};
.con _cps_54993 _54994::[_54998: «2; %math.F (52, 11)», _54996: .Cn .Idx 2]@(0:(.Idx 2)) = {
    .let _54999: .Idx 2 = %core.bitcast «2; %math.F (52, 11)» .Idx 2 _54998;
    _54996 _54999
};
.lam eta__54897 _55002: «2; %math.F (52, 11)»: .Idx 2 = {
    %direct.cps2ds_dep («2; %math.F (52, 11)», Uf_54920) _cps_54993 _55002
};
.lam eta__55004 _55005: «2; %math.F (52, 11)»: .Idx 2 = {
    %direct.cps2ds_dep («2; %math.F (52, 11)», Uf_54920) _cps_54993 _55005
};
.con .extern stuff ... = {
    ...
    .let _55110: %mem.M = %bug.map_reduce 1 ‹2; (4, (1, 2, 15, 15), (%math.F (52, 11), 0:(%math.F (52, 11)), 4607182418800017408:(%math.F (52, 11)), %math.abs (52, 11) 0, eta__54897, eta__55004))› apply_55089 (_55104#0:(.Idx 2), _55104#1:(.Idx 2), _55104#1:(.Idx 2));
    return_54864 _55110
};

(Side note: eta__54897 and eta__55004 look like they should be unified as equal.)

The call to _cps_54993 is now tackled by cps2ds.
This is done (similar to the fun stuff) by calling it in cps with continuations for the rest.
Therefore, the body of eta__55004 is replaced by _cps_54993 ....
This, however, results in the type error above.

The issue lies in the assumptions of the pass:
As direct style functions can not generate code, all functions should fall into three categories:

  • Lowered by ds2cps into a cps function (that is just wrapped into a cps2ds call at its old position)
  • statically resolvable
  • higher order function that ends in a cps function

eta__54897 is used a as higher order argument contradicting these assumptions.

@leissa
Copy link
Member Author

leissa commented May 23, 2023

A makefile might be a good idea to simplify using the cmake commands. (Especially since the lit commands as described in the documentation do not work me right now).

Reproducing bugs is a major pain and one of the reasons, I'm not pursuing them as tenaciously as I should. I should open an issue to ease the process. Basically by providing a bunch of scripts and you only have to press one button to reproduce a bug. That would definitely help a lot (Edit: see #224).

(Side note: eta__54897 and eta__55004 look like they should be unified as equal.)

Rn, Thorin doesn't have a reason to unify them. If you had (eta__54897, eta__55004), we would hopefully see ‹2; eta__54897›. If we want to get rid of duplicates more aggressively, we would need a proper GVN or so.

Coming to the actual issue:
It seems to me, the real solution here, would be to detect this and throw an appropriate error message to notify anyone running into this issue, what needs to be done to fix this. There are a couple problems like this with several passes/phases within Thorin that make some implicit assumptions and will silently crash, if these are not met.

@NeuralCoder3
Copy link
Collaborator

NeuralCoder3 commented May 26, 2023

b398b7d warns if direct style functions remain and ignores them.
The example now goes through without crashing.
Although it will crash during llvm code generation.
(No fix is suggested as there is not one way to resolve all possible cases (e.g. non-callee direct style functions))

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

No branches or pull requests

2 participants