Tail recursion

[vidsinghal]

WIP: Add a pass to analyze functions that are tail recursive and add mutable cursors.

In case of a mutable cursor, we do not need to store a (Cursor, Cursor) for the start 
and the end of a packed value. 
In case a location referes to a mutable cursor, we can just keep a Mutable Cursor for
that location. 

Mutable Cursor version for add1Tree

-- char* 
type Cursor = Ptr Char 

-- char**
type MutableCursor = Ptr Ptr Char

add1 :: Cursor -> Cursor -> ()
add1 lout lin = 
  let tag = readTag lin 
   in case tag of 
        Leaf  -> let n = readInt (tag + 1) 
                     () = writeTagMutable lout Leaf
                     () = BumpMutableCursor lout 1  
                     () = writeIntMutable lout (n+1)
                     () = BumpMutableCursor lout 8
                   in ()
        Node -> ...

Packed input still becomes a read cursor. 
Packed output values become just one mutable cursor.

[ulysses4ever]

Cool work, congrats!

Can you remove the binary (*.exe) files from the patch? Perhaps, the .s assembly file too. They can be generated, right? And they have a high chance of failing on a machine other than yours. Hence, they should not be committed.

There should be a test checking that TCO happens in a simple example. Let me know if you need help creating such a test: we can discuss and find a way: worst case, we can grep the assembly. We just need to figure out where to fit this in the existing test suite.

[ulysses4ever]

Is this a temporary change to see how far CI advances, or do you intend it to stay that way?

[vidsinghal]

just temporary

[ulysses4ever]

Can we have one empty line between definitions? (Same below.)

[ulysses4ever]

The last _ should follow the same convention as other unused binding, i.e. _blah where blah gives some meaning to the binding. Same below

[ulysses4ever]

You're using binding names with the _ prefix, which are usually meant to be unused: _blah is how you give a name to a binding otherwise unused to improve readability and avoid the unused-binding warning. I suggest removing _ from the names that are actually used. There was an old GHC ticket that suggested making this a warning but it didn't get traction.

[ulysses4ever]

But better yet, you should simply use record update syntax here: funTy { tailRecType = TC } -- instead of the whole let.

[ulysses4ever]

This branch looks very similar to the previous one. I suggest factoring out common code. It's very hard to spot a difference and hence, to miss a typo.

[vidsinghal]

what other branch?

[ulysses4ever]

You got to this comment after modifying the code, so it's tricky. The same duplication is still in the current version here:

gibbon/gibbon-compiler/src/Gibbon/Passes/MarkTailCalls.hs

Lines 34 to 40 in cef8b0d

	funTy' = (ArrowTy2 locVars' arrIns _arrEffs arrOut _locRets _isPar)
	in return $ FunDef funName funArgs funTy' funBody' funMeta {-dbgTraceIt (sdoc (tailCallTy, funName, funTy')) dbgTraceIt "a" dbgTraceIt (sdoc (tailCallTy, funName, funTy')) dbgTraceIt "a" -}
	else if tailCallTy == TC
	then
	let (ArrowTy2 locVars arrIns _arrEffs arrOut _locRets _isPar) = funTy
	funTy' = (ArrowTy2 locVars arrIns _arrEffs arrOut _locRets _isPar)
	in return $ FunDef funName funArgs funTy' funBody' funMeta {-dbgTraceIt (sdoc (tailCallTy, funName, funTy')) dbgTraceIt "b" dbgTraceIt (sdoc (tailCallTy, funName, funTy')) dbgTraceIt "b" -}

Are you aware of the record update syntax in Haskell? E.g.:

So, in general, to update the field x in a value y to z, you write y { x = z }

https://en.wikibooks.org/wiki/Haskell/More_on_datatypes#Named_Fields_(Record_Syntax)

[vidsinghal]

Oh ok conditional branch, branch was ambiguous at first.

[ulysses4ever]

Oh, my bad!