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

WIP: Test tools and tests for lowering (pass 1 - syntax desugaring) #32201

Closed
wants to merge 56 commits into from

Conversation

c42f
Copy link
Member

@c42f c42f commented May 31, 2019

I've been looking into what it would take to port lowering from flisp to julia. It's a reasonable chunk of work so I'm starting with test infrastructure to make the actual work easier.

Here's the first part of that:

  • Some small changes to allow calling the syntax desugaring pass from julia and getting the desugared AST back in a useful form.
  • The start of some test tooling so that it's easy to write tests for desugared ASTs as normal julia ASTs. The idea is to have a bidirectional translation of some of the exotic Expr heads into a form which is easy to read and write as normal julia code.
  • An Expr diffing tool, using the dump format
  • A few tests for the current flisp-based lowing which use these tools.

Once there's some reasonable coverage here it should be a lot more straightforward to start rewriting some of the old code in julia.

Clearly there's a lot of forms which are not yet tested, but I'm posting this for early feedback on the general approach.

@c42f c42f requested a review from Keno May 31, 2019 08:32
@c42f c42f added compiler:lowering Syntax lowering (compiler front end, 2nd stage) test This change adds or pertains to unit tests labels May 31, 2019
@Keno
Copy link
Member

Keno commented Jun 3, 2019

I love everything about this. Of course @JeffBezanson should also take a look.

@c42f
Copy link
Member Author

c42f commented Jun 3, 2019

Great, I'll keep pursuing this until I have decent coverage of the desugaring pass.

I've noticed that in some cases (eg, lowering of functions) there's quite a lot of novel Expr heads. The current plan is just to express the test reference ASTs with interpolation for those cases to keep the tests explicit. I'll keep some light translation for things like Expr(:top) and Expr(:core).

@JeffBezanson
Copy link
Sponsor Member

This is awesome, and very brave of you to take this on :)

There is one significant refactoring I think we should do before porting, which is basically to eliminate the file macroexpand.scm by integrating macro hygiene with the usual scope resolution pass. That will let us drop around 500 lines of awkward code.

Some of the lowering stuff will be annoying to unit test, since the outputs can be implementation-specific, i.e. not canonical or meaningful in usual julia. A new implementation would probably do things differently.

I'm also not sure the approach of translating lowered forms back to Exprs for testing will be general enough. Ideally, we should have notation for everything we emit, and not rely on projecting everything back to a different object model we happen to have notation for (julia Exprs). In fact we should probably add a simple s-expression parser so we can write out structures clearly. It would also be nice to have a notation for IR, e.g. similar to LLVM syntax, which would be ideal for testing the end result of lowering.

@c42f
Copy link
Member Author

c42f commented Jun 5, 2019

very brave foolish of you to take this on

There, I fixed that for you :-)

I'm also not sure the approach of translating lowered forms back to Exprs for testing will be general enough.

My hope was that it's almost general enough for the desugaring pass at a pinch. However, it's already showing signs of trouble as you can see in the amount of $(Expr(:exotic_head, ...)) which occur in the reference forms, especially after lowering functions and loops. And it's slightly broken for lambda forms which include some decidedly non-Expr data. I worked around that for now but it's ugly.

Perhaps one could argue that the desugaring pass should eventually be defined by the property that it turns valid julia code into valid julia code? Then further analyses would be factored into the next pass or two.

Ideally, we should have notation for everything we emit, and not rely on projecting everything back to a different object model we happen to have notation for (julia Exprs). In fact we should probably add a simple s-expression parser so we can write out structures clearly. It would also be nice to have a notation for IR, e.g. similar to LLVM syntax, which would be ideal for testing the end result of lowering.

Some of the lowering stuff will be annoying to unit test, since the outputs can be implementation-specific, i.e. not canonical or meaningful in usual julia.

Yeah too much implementation-specific detail will mean refactoring the tests all the time. But on the other hand it's currently quite hard to see/test what the passes do in isolation. I'd like some happy medium where we identify two or three types of well defined data consumed and produced by passes. I think that's what you're suggesting?

I'm imagining:

  • Expr
  • --(desugar)-->
  • Expr (maybe)
  • --(scope resolution)-->
  • s-expression?
  • --(closure conversion)-->
  • s-expression?
  • --(linearize)-->
  • Julia IR

Are s-expressions the right way to express those intermediate stages? Obviously they're natural right now because everything is written in flisp. But in a julia version of that code I'd imagine there would be a julia type for Lambda, etc? Obviously such objects could still be serialized as s-expressions and that might be good enough for testing purposes?

@c42f c42f closed this Jun 5, 2019
@c42f c42f reopened this Jun 5, 2019
@c42f
Copy link
Member Author

c42f commented Jun 5, 2019

Oops, closed this and posted an incomplete comment by mistake... deleted.

@c42f
Copy link
Member Author

c42f commented Jun 5, 2019

Well, I've added a toy S-Expression parser and tried out how it would look to write the test cases in that.

It's not bad, and sometimes it's a good improvement, for example testing while loops:

    @test_desugar(while cond
                      body1
                      continue
                      body2
                      break
                      body3
                  end,
        $(Expr(Symbol("break-block"), Symbol("loop-exit"),
               Expr(:_while, :cond,
                    Expr(Symbol("break-block"), Symbol("loop-cont"),
                         :(let
                             body1
                             $(Expr(:break, Symbol("loop-cont")))
                             body2
                             $(Expr(:break, Symbol("loop-exit")))
                             body3
                         end)))))
    )

vs, in the current sketch:

    @test_desugar_sexpr(while cond
                            body1
                            continue
                            body2
                            break
                            body3
                        end,
        "(break-block loop-exit
           (_while cond
             (break-block loop-cont
               (scope-block
                 (block
                    body1
                    (break loop-cont)
                    body2
                    (break loop-exit)
                    body3)))))"
    )

I do find the idea that the desugaring pass would strictly map julia syntax to julia syntax rather appealing. But in practice the current desugaring pass doesn't do this (as seen above) and it's not clear to me how it could be made to do so while also preserving the structure of the current system. Thoughts?

@JeffBezanson
Copy link
Sponsor Member

Obviously such objects could still be serialized as s-expressions and that might be good enough for testing purposes?

Yes, I'd just rather write (foo x) instead of $(Expr(:foo, x)). s-expressions are a compact notation for this kind of structure whether you're using lisp or not. I really like the idea of having an s-expr parser around; maybe we should fully revive LispSyntax.jl ?

Perhaps one could argue that the desugaring pass should eventually be defined by the property that it turns valid julia code into valid julia code?

That isn't really possible, since part of the point of the process is to introduce concepts that don't exist at the source level, like SSA values. Another example is new, which we don't want to be valid julia code.

@c42f
Copy link
Member Author

c42f commented Jun 7, 2019

Well I revived LispSyntax over at swadey/LispSyntax.jl#21. Also LispREPL for good measure.

@c42f
Copy link
Member Author

c42f commented Jun 7, 2019

Having rewritten a few examples I prefer the pseudo julia code as reference expressions for the more trivial lowerings. I think I might mix and match; the cost isn't too bad at ~100 lines of infrastructure code and it makes the tests more approachable.

Eg:

    @test_desugar a.b    Top.getproperty(a, :b)
    @test_desugar a.b.c  Top.getproperty(Top.getproperty(a, :b), :c)

    @test_desugar(a.b = c,
        begin
            Top.setproperty!(a, :b, c)
            maybe_unused(c)
        end
    ) 

vs

    @test_desugar a.b    sx"(call (top getproperty) a (quote b))"
    @test_desugar a.b.c  sx"(call (top getproperty)
                              (call (top getproperty) a (quote b))
                              (quote c))"

    @test_desugar(a.b = c,
        sx"(block
             (call (top setproperty!) a (quote b) c)
             (unnecessary c))"
    )

@StefanKarpinski
Copy link
Sponsor Member

StefanKarpinski commented Jun 7, 2019

Maybe a SExprs stdlib?

@c42f
Copy link
Member Author

c42f commented Jun 7, 2019

A stdlib would make sense.

In addition to LispSyntax.jl, there's @TotalVerb's https://github.com/TotalVerb/SExpressions.jl which looks rather nice, I need to have a closer look at that. Being based on racket, the syntax is a lot closer to flisp too (that's nice for me; flisp is the only lisp I "know" :-) ).

@StefanKarpinski
Copy link
Sponsor Member

SExpressions is probably a better name too.

@chethega
Copy link
Contributor

That isn't really possible, since part of the point of the process is to introduce concepts that don't exist at the source level, like SSA values. Another example is new, which we don't want to be valid julia code.

But :new is definitely relevant and valid for e.g. deserialization code that does not want to call inner constructors. Likewise, Pi-nodes could be useful (type-assume instead of type-assert). I don't see why SSA values are a thing that does not exist at the code level: SSA-ness is a property of the code, not of our representation of the code.

It would be super elegant if lowering passes could be projections (idempotent), that successively restrict the set of available language features. Lowering passes can obviously not be strictly idempotent, because we want to change the data layout in between, but that is cosmetics (e.g. refer to SSA-Values and labels by integers instead of names, store the fact that SSA-Values have already been checked against use-before-init, store the fact that exception handlers are sound, i.e. the stack of exception handlers at each code position is independent of the execution path we took to get there, line-number info, etc).

@c42f
Copy link
Member Author

c42f commented Jun 25, 2019

Well I've spent far too much thinking about SExpressions vs Julia syntax now but I think I've finally found an acceptable form for the reference ASTs here.

The main problem with the unquoted interpolation $(Expr(exotic_head, args...)) for representing lowered Expr heads is that it forces args... to be quoted again. So reading these expressions is very confusing because you're constantly switching between quoted vs unquoted forms depending on the nesting.

Using S-Expressions would be one way around this. I tried it but found it ultimately a bit dissatisfying because so many of the reference expressions are almost-but-not-quite Julia surface syntax which is usually more compact and familiar than the S-Expression form.

The solution is to introduce an @Ex @Expr macro in the reference forms such that foo(@Expr(:somehead, blah(x))) expands to foo($(Expr(:somehead, :(blah(x))))). The important thing here is that there's no need to mentally switch between reading quoted vs unquoted code.

For example, lowering of this while loop

while cond
    body1
    continue
    body2
    break
    body3
end

can be written as

@Expr(:break_block, loop_exit,
      @Expr(:_while, cond,
            @Expr(:break_block, loop_cont,
                  @Expr(:scope_block, begin
                            body1
                            @Expr :break loop_cont
                            body2
                            @Expr :break loop_exit
                            body3
                        end))))

@c42f
Copy link
Member Author

c42f commented Jun 25, 2019

@Ex could also be more generally useful: Say we renamed @Ex as @Expr and make show_unquoted emit that rather than the interpolation $(Expr(...)) for unusual expression heads.

I think this would be an improvement in readability. Worth pursuing? Done

c42f added 5 commits June 25, 2019 11:56
Start building up a test suite for lowering so that we can port the
lowering code to julia with efficiency and confidence.
Tools:
* Renumber gensyms
* macro to test desugaring errors
* catch flisp exceptions

Tests:
* short circuit
* ternary
* adjoint
* broadcast
* keyword args
* inplace update operators
* destructuring assignment
+ Test assignment errors
@vtjnash
Copy link
Sponsor Member

vtjnash commented May 29, 2021

I think some of this sounded useful, and should have been a separate PR so that it didn't get stuck inside a massive change. Closing as I'm not sure what part is useful now, and I think we aren't going in this direction currently for lowering.

@vtjnash vtjnash deleted the cjf/lowering-in-julia branch May 29, 2021 04:19
@c42f
Copy link
Member Author

c42f commented May 31, 2021

This was a huge pile of work and I had hoped to resurrect it at some stage. Perhaps pull out some useful parts.

I think we aren't going in this direction currently for lowering.

Which direction are you going in?

@c42f
Copy link
Member Author

c42f commented May 31, 2021

When I say a huge pile of work... it was nearly a 4000 line patch here. Yikes.

@vtjnash
Copy link
Sponsor Member

vtjnash commented May 31, 2021

I'm not sure we're going in any direction right now, since it has been exactly 2 years today since this was opened 😂. But also just looking back at the discussion in the issue, e.g. #32201 (comment)

GitHub will never delete a branch once it is a closed PR (refs/pull/#/head). I agree it sounds like there is useful pieces in this.

@c42f
Copy link
Member Author

c42f commented Jun 1, 2021

I'm not sure we're going in any direction right now, since it has been exactly 2 years today since this was opened

Oh right. Just wondered whether there was some other plan, or whether something like this is still considered a reasonable idea (if only I can find time for it.)

@c42f
Copy link
Member Author

c42f commented May 21, 2024

It's finally happening! In a very different and more capable form than this old code, but it's happening:

https://github.com/c42f/JuliaLowering.jl

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compiler:lowering Syntax lowering (compiler front end, 2nd stage) test This change adds or pertains to unit tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants