Skip to content

Small improvements to type-based optimizations (array, lazy)#712

Merged
alainfrisch merged 12 commits intoocaml:trunkfrom
alainfrisch:improve_typeopt
Jul 27, 2016
Merged

Small improvements to type-based optimizations (array, lazy)#712
alainfrisch merged 12 commits intoocaml:trunkfrom
alainfrisch:improve_typeopt

Conversation

@alainfrisch
Copy link
Copy Markdown
Contributor

This PR improves the type-based optimization logic for array and lazy.

For arrays: this is about knowing that the array cannot hold floats, or can only hold ints. Improvements brought by the PR:

  • Arrays on _ Lazy.t or bytes (in -safe-stringmode) are now properly detected to never hold floats.
  • Reuse the machinery introduced for the @@immediate attribute, which detects more arrays as holding only ints (closed polymorphic variants with only constant constructors, or abstract types marked with the attribute).

For lazy: this is about knowing that lazy x can be compiled just as x without introducing a forward block. The forward block is needed only if x can be a float or lazy value. Improvements:

  • Detect bytes (in -safe-stringmode).
  • Expand abbreviations.
  • Reuse the machinery for the @@immediate attribute.
  • Logic moved to Typeopt (where it really belongs) instead of Translcore.
  • Share this logic with the one for arrays (the only difference being the case of the lazy_t abstract type): this brings the more clever support for concrete data types (records, sums) which are known to be neither floats nor lazy values, and will also better interact with Unboxed types #606.

@alainfrisch alainfrisch changed the title Small improvements type-based optimization Small improvements to type-based optimizations (array, lazy) Jul 25, 2016
@chambart
Copy link
Copy Markdown
Contributor

As you are apparently the one interested about lazy performance, do you consider that it would matter to be able to remove some more of those forward blocks during inlining ?
I was thinking of situations where the concrete type is hidden but some information about
the value are available:

module A : sig
  type t
  val v : t
end = struct
  type t = int
  let v = 1
end

let x = lazy A.v

@alainfrisch
Copy link
Copy Markdown
Contributor Author

As you are apparently the one interested about lazy performance

Not very strongly, actually. It's just that the bug with flambda prompted me to look at it...

But yes, removing some extra forward blocks could be useful. I was thinking about just calling Lazy.from_val (but optimizing the tag extraction which is currently a C primitive) instead of creating the forward block explicitly (in flambda mode!) and letting flambda inlines it and either remove useless checks or keep runtime dispatch (which might be cheaper than creating the forward block when we are statically unsure whether it is needed -- to be checked).

This commits reuses the machinery introduced for [@@Immediate] in
order to improve the detection of the Pintarray case.  This detects
some new cases such as closed polymorphic variants with only constant
constructors, or abstract types marked with [@@Immediate].
…lock

This avoids introducing a forward block in cases such as:

   type t = int -> int
   let f (x : t) = lazy x
This commits reuses the machinery introduced for [@@Immediate] in
order to improve the detection of cases where the forward block is not
needed for compiling `lazy x`.  This detects some new cases such as
closed polymorphic variants with only constant constructors, or
abstract types marked with [@@Immediate].
The logic for arrays was more clever than the one for compiling 'lazy
x' without a forward block.  In particular, it knows that concrete
data types (records, variants, open variants) cannot be floats (they
cannot be lazy either).  This commits reuses this logic for deciding
if 'lazy x' must introduce a forward block.  Now, the only difference
between the two cases is the abstract built-in type 'lazy_t'.

This commits also robustifies the classification logic to explicitly
deal with all kinds of types; and it removes an unused function.
@alainfrisch
Copy link
Copy Markdown
Contributor Author

Does anyone want to review this change?

@chambart chambart self-assigned this Jul 27, 2016
transl_exp e
| Texp_constant(Const_float _) | Texp_ident _ ->
| Texp_constant(Const_float _) ->
Lprim(Pmakeblock(Obj.forward_tag, Immutable, None),
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Please add a comment explaining that this is ok since this won't be shortcuted for floats.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done (note that this PR did not change this behavior, @mshinwell already noted that the float case does not require the opaque hack).

@chambart
Copy link
Copy Markdown
Contributor

This seems ok, and it cleans some duplications.

Maybe the P*array should be renamed. This does not represent an array anymore.

@alainfrisch
Copy link
Copy Markdown
Contributor Author

Maybe the P*array should be renamed

Good point, I've introduced a new variant type for the classification.

Thanks for the review!

@alainfrisch alainfrisch merged commit c621713 into ocaml:trunk Jul 27, 2016
camlspotter pushed a commit to camlspotter/ocaml that referenced this pull request Oct 17, 2017
Small improvements to type-based optimizations (array, lazy)
EduardoRFS pushed a commit to esy-ocaml/ocaml that referenced this pull request Dec 17, 2021
otherlibs: Unix.kill should check for pending signals
stedolan pushed a commit to stedolan/ocaml that referenced this pull request Sep 21, 2022
stedolan pushed a commit to stedolan/ocaml that referenced this pull request Sep 21, 2022
ce76e02 flambda-backend: Bugfix for type_application (ocaml#746)
44f3afb flambda-backend: PR580 for main branch (ocaml#743)
b851eaa flambda-backend: Backport first part of ocaml/ocaml PR10498 (ocaml#737)
fafb4bd flambda-backend: Fix return mode for eta-expanded function in type_argument (ocaml#735)
c31f6c3 flambda-backend: Fix treatment of functions called [@nontail] (ocaml#725)
847781e flambda-backend: Fix build_upstream post-PR703 (ocaml#712)
bfcbbf8 flambda-backend: Extend Pblock value kind to handle variants (ocaml#703)
b2cab95 flambda-backend: Merge ocaml-jst
a6d6e0e flambda-backend: Merge ocaml-jst
88a4f63 flambda-backend: Use Pmakearray for immutable arrays (ocaml#699)
eeaa44b flambda-backend: Install an ocamldoc binary (ocaml#695)
48d322b flambda-backend: Ensure that GC is not invoked from bounds check failures (ocaml#681)
4370fa1 flambda-backend: Review changes of term directory (ocaml#602)
65a4566 flambda-backend: Add code coverage using bisect_ppx (ocaml#352)
63ab65f flambda-backend: Bugfix for primitive inclusion (ocaml#662)
7e3e0c8 flambda-backend: Fix inclusion checks for primitives (ocaml#661)
96c68f9 flambda-backend: Speed up linking by changing cmxa format (ocaml#607)
1829150 flambda-backend: Bugfix for Translmod.all_idents (ocaml#659)

git-subtree-dir: ocaml
git-subtree-split: ce76e02
sadiqj pushed a commit to sadiqj/ocaml that referenced this pull request Feb 21, 2023
EmileTrotignon pushed a commit to EmileTrotignon/ocaml that referenced this pull request Jan 12, 2024
Point to the mirror of the caml-list archive provided by
Leah Neukirchen at https://inbox.vuxu.org/caml-list/ as
https://inbox.ocaml.org/caml-list/ is down.

See:

* ocaml/ocaml.org#550
* ocaml/infrastructure#15

Co-authored-by: Cuihtlauac ALVARADO <cuihtmlauac@tarides.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants