Flambda manual chapter by mshinwell · Pull Request #503 · ocaml/ocaml

mshinwell · 2016-03-11T10:42:53Z

User documentation about Flambda for the manual.
If you would like to read this without building the manual, there is a temporary link here:
https://ocaml.janestreet.com/ocaml-core/flambda_manual/

This doesn't constitute developer documentation. That is intended to be provided by the source code itself and the comments in that code. We will try to do some more improving of comments before the 4.03 release.

In this GPR there is a hack in fix_index.sh (so it would work for me) which I shall revert before merging this.

bobot · 2016-03-11T12:05:11Z

manual/manual/cmds/flambda.etex

+  iter foo [1; 2; 3]
+\end{verbatim}
+The argument {\tt f} to {\tt iter} is invariant so the function may be
+specialised:


Why do you call that a specialization even if no argument have been substituted? Even if it is not the reality, for the sake of the explanation is it not better to write:

let do_iter foo = let rec iter' l = match l with | [] -> () | h :: t -> foo h; iter' t in iter' [1; 2; 3]

The comment in the code shows the specialisation.
I will add another paragraph showing what happens next (the indirect call being replaced by the direct call, followed by removal of the unused argument f).

bobot · 2016-03-11T12:46:47Z

This chapter is very interesting to read, thank you!

edwintorok · 2016-03-11T12:49:30Z

manual/manual/cmds/flambda.etex

+this function should probably not be inlined systematically: if {\tt b}
+is unknown, or indeed {\tt false}, there is little benefit to trade off
+against a large increase in code size.  In the existing non-Flambda inliner
+this isn't a great problem because chains of inlining were cut off farly


Typo: s/farly/fairly/

edwintorok · 2016-03-11T12:59:03Z

Impressive amount of work (both flambda and this manual).
There are some things that are unclear, perhaps these could be briefly mentioned in the manual as well?

What happens with bounds checks?
Does simplification eliminate them when it is statically known that they are never exceeded?
And would this only work for constants or symbolic expressions as well?
E.g. if I have a loop from 0 to String.length s - 1 and access each character in the string, will Flambda be able to remove the bounds check?
Can Flambda use assert false, or assert cond as hints for its optimizations?
Would dead code elimination allow Flambda to skip optimizing functions that are not used inside modules?
E.g. if a module is created by applying functors, and not all functions are used, and module is not visible outside of the .ml
Is it possible to defer Flambda to link time and avoid generating native code (i.e. GCC/Clang -flto)?
I would guess no, although considering that "inlining of functions from previously-compiled units will subject their code to the optimisation parameters of the unit currently being compiled, rather than
those specified when they were previously compiled." could I tell Flambda to recursively recompile all functions used based on their Flambda form in the .cmx at link time?
Of course there is a tradeoff here: optimizing/generating code at link time is not easily parallelizable, OTOH it can avoid optimizing/generating code for functions/modules that are not used at all.

edwintorok · 2016-03-11T13:06:33Z

manual/manual/cmds/flambda.etex

+Consult the {\em Glossary} at the end of this chapter for definitions of
+technical terms used below.
+
+\section{Command-line flags}


Do these flags only have effect at compile time, or should they be used at link time too?

Theoretically they have an effect at link time because they apply to the code in the startup file. However in practice that's probably not relevant.

mshinwell · 2016-03-11T13:10:33Z

The last two questions are in fact related, so let's address those first. There isn't any support at the moment for saying things like "function foo isn't exposed in the interface of this module and isn't used inside, so we'll delete it before optimisation". It's possible something like that could be thought about in the future though.

I have plans for a whole-program mode which will recompile at link time based on the intermediate representations in the .cmx files (with all .cmx files being required). We could have a flag to suppress normal object file generation when planning to link in that mode. The main advantage this will give is that dead code elimination will happen at the Flambda stage basically automatically. Proposals for other means of large-scale dead code elimination are highly involved.

mshinwell · 2016-03-11T13:16:44Z

I think the answer about bounds checks is that they won't be eliminated statically since they are expanded after Flambda. (I'm not completely sure about this, but I think it is the case.)

There isn't any special handling based on "assert".

lindig · 2016-03-11T13:28:41Z

I agree with @edwintorok, this is impressive. I think adding a section with (automatically generated) benchmark numbers (optimisation, code size, performance) would provide valuable context as it is difficult to imagine the effect of different optimisations.

mshinwell · 2016-03-11T13:38:55Z

The problem is that it so heavily depends on what the code actually is that generalised benchmark numbers may be misleading. I think we should perhaps leave something like that until 4.04, when we will have received more information on general use cases in the wild.

mshinwell · 2016-03-11T14:49:37Z

Suggestions received from William via email, which I shall look at:

"20.1 :
please give the command or procedure that tells whether the ocaml we have (through an already existing installation, for example debian) has flambda activated or not

"Detailed descriptions of each flag are given in the sections that follow." : I would replace by : "Terms and concepts used here are better described in the following sections".
Also, I don't know if that would make the reading worse or not, but I would replace each defined term like "call site", "specialised", "inline" by a pointer to the glossary. Similarly, you should reference for each options the related chapters (the page is big, and it is not easy to have a full view of what is going to be explained, and we might also miss some chapters related to an option).

20.2 :
-remove-unused-arguments
"Remove unused function arguments even when the argument is not specialised." :
do you mean "remove unused argument that are not specialised, in addition to unused arguments that are specialised" ?
then what is the trade-off behind this option ? why is it not activated by default ?

-unbox-closures
make pointers to the glossary and to the related chapters (20.9.3).

"advanced options" : do you mean it is even less used that "less commonly-used options" ?

"The set of command line flags relating to optimisation should typically be specified to be the same across an entire project." why not give a typical ocamlbuild command. ? In the mean time, I see you are from Janestreet, so you would not do that as you use other tool ...

"Flambda-specific flags are silently accepted " : as it is silent only for benchmark purposes, why not make warnings by default, and add an option to make them silent when performing benchmanking ?

"-rounds" : refer to the related chapter 20.2.1

"If the first form is used, the value will apply to all rounds.If the second form is used, zero-based round integers specify values which are to be used only for those rounds. " : what is first/second forms ? Maybe reformulating this with examples would be clearer.

Glossary
"call site" : I guess it is the point where the function is used, rather than the point where the function is defined
"closure" : This is really hard to understand. Maybe more explanations, or use of less technical terms ?
"specialised argument" : you should refer to the "specialised" chapter/section."

bluddy · 2016-03-11T16:41:27Z

manual/manual/cmds/flambda.etex

+based on the dataflow between invariant arguments.
+
+\paragraph{Example:} Having been given the following code, the compiler
+will inline {\tt loop} into {\tt f}, and then observe {\tt env}


Drup · 2016-03-19T14:21:11Z

Remarks:

I'm not really fond of how the description of options is at the top, disconnected from the section that actually explains what they do. When you read the explanation, you have to backtrack to see the name of the options. I really think you should at least add many forward/back references.
Description of options should say what they take as arguments. I had to look at the default to know those were mostly integers.
In one of the code examples:

let bar x =
    x * 3
  [@@inline never]

can also be written :

let[@inline never] bar x =
    x * 3

Same thing for the module. I'll let you decide if it's better or not. :)

It looks like flambda can detect if the argument of a function is not used at all in the compilation unit. Would this trigger a warning if the function is not exported ? It sounds like a useful warning.
About non-escaping references, would it be useful to have an attribute for that, [@noescape], that would produce an error if it's not respected ? (It sounds useful, but you may know better).

jorisgio · 2016-03-23T15:29:01Z

manual/manual/cmds/flambda.etex

+application and takes an integer argument.  Each time the inliner sees
+the attribute it behaves as follows:
+\begin{itemize}
+\item If $n$ is zero or less, nothing happens.


The compiler accepts [@unrolled] but it is not clear what effect it has. It looks like it is equivalent to [@unrolled 0] ? It should probably be documented here

@Drup: comments on the manual noted.

It seems that the warning about unused arguments you suggest would be easier implemented prior to Flambda. (This presumably concerns cases involving recursive functions, since there is already a warning for simple cases.) Flambda ends up making arguments unused, and we probably wouldn't want to trigger the warning for those. For example [Remove_unused_arguments] will remove a single unit argument to a function, forming a zero-arity function, and you wouldn't want to warn about that...

The [@NoEscape] attribute sounds like a good idea. Two places in the compiler ([Simplif] and [Ref_to_variables]) would need updating. We would be pleased to receive a patch from you. :)

mshinwell added this to the 4.03.0 milestone Mar 11, 2016

bobot reviewed Mar 11, 2016
View reviewed changes

edwintorok reviewed Mar 11, 2016
View reviewed changes

bluddy reviewed Mar 11, 2016
View reviewed changes

Drup mentioned this pull request Mar 19, 2016

AMA about Flambda #517

Closed

jorisgio reviewed Mar 23, 2016
View reviewed changes

mshinwell force-pushed the flambda_doc branch from 3461f30 to 8427dc7 Compare March 31, 2016 14:35

Conversation

mshinwell commented Mar 11, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bobot commented Mar 11, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

edwintorok commented Mar 11, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mshinwell commented Mar 11, 2016

Uh oh!

mshinwell commented Mar 11, 2016

Uh oh!

lindig commented Mar 11, 2016

Uh oh!

mshinwell commented Mar 11, 2016

Uh oh!

mshinwell commented Mar 11, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Drup commented Mar 19, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants