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

[Blog] Caiman Frontend Project #430

Open
wants to merge 2 commits into
base: 2023fa
Choose a base branch
from

Conversation

stephenverderame
Copy link
Contributor

Closes #402

Copy link
Owner

@sampsyo sampsyo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wow! Really nice work on this writeup, @stephenverderame. You have done an absolutely fantastic job explaining some tricky concepts in a very complicated language. The frontend itself is looking good, and the writeup here is excellent. Thanks for the very clear exposition of the problems and their solution. 👏

I have included a few low-level suggestions for the text!


![Architecture](HlcArch.svg)

The HIR I designed is somewhat similar to BRIL, however, its canonical
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BRIL -> Bril


![Architecture](HlcArch.svg)

The HIR I designed is somewhat similar to BRIL, however, its canonical
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd replace the first , with a ;.
https://capra.cs.cornell.edu/styleguide/#however


Once the AST is in CFG form, we perform a few local transformations as "preprocessing".
Namely, we convert all operators to their typed forms
(so `add` becomes `_add_i64_i64` for adding two `i64`s) and convert all
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is usually called "name mangling."

Comment on lines +188 to +191
One could imagine employing the progressive lowering ideas of CompCert and MLIR
to separate the HIR into two HIRs, one before these transformations and
a second representation afterward. I didn't go this route because it would
involve decent code duplication for the definitions of the two IRs.
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point! I'd use your strategy in this situation, fwiw.


It's still not pretty on account of the current need to match pieces of
the implementation back to the spec almost everywhere (denoted with the `@`)
and keep expressions unnested. However, the schedule in this example has the following
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you think it would be possible to include a brief explanation of what the types mean? That is, in bool @ node(main.b), how should we read each part? How about in [node(main.r)-usable, none(space)-save]? Including a few direct descriptions like these might make it easier for readers to parse the example code.

the continuation of each basic block is. In the above example, the
continuation of `foo` is `foo2` and the continuation of `foo2` is `foo3`.
`foo4`, `foo5`, `foo6`, and `foo7` are at the maximum depth in this example, so
when they're done, control is going to pass back to their parent's continuation.
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Based on the examples, am I correct in concluding that the continuation for a given block $b$ is the "join point" following $b$? Or, more formally, that it's $b$'s immediate post-dominator?

Comment on lines +374 to +376
Luckily, we already know what the continuation of every block is because
we determined this information when we generated the basic blocks from the
source program.
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is very tricky! Interesting observation here.

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.

Project Proposal: Caiman Frontend
2 participants