[m-rev.] for preliminary review: Add option to break trans-opt dependencies.

[Zoltan Somogyi]

2022-12-19 17:48 GMT+11:00 "Peter Wang" <novalazy at gmail.com>:
> Here is a patch to improve parallel making of .trans_opt files.
> The naming of things is rather clunky.

My prelim review is attached.

> Here are times for "mmake -j32 trans_opts" in the library directory,
> using the attached mer_std.make-trans-opt-deps file:
> 
> - without the new option
> 
>     real        0m39.742s
>     user        2m11.086s
>     sys         0m5.166s
> 
> - with --make-trans-opt-deps-spec
> 
>     real        0m18.700s
>     user        1m42.337s
>     sys         0m4.331s

That is nice. What do you think is the cause of the
reductions in user and sys times? Is it just fewer
.trans_opt files being read, or is there some other factor as well?
 
> The information of interest in the mer_std.make-trans-opt-deps file is
> all in the %-commented out lines.

What is the purpose of the /* */ commented out lines?

For the bitmap regression and possibly others, we could fix it
by moving is_error, throw_on_error and their variants, along with
the types specific to their operations, to a new module. We could
keep forwarding predicates, marked obsolete, in io.m for now,
but redirect all references from the Mercury stdlib now.

> For a given source module, we want to
> prevent the compiler reading another module's .trans_opt file.
> Therefore, I'm thinking the file should state the negative dependencies
> directly, e.g.
> 
>     foo - {
>         not bar,
>         not baz
>     }

I am not a fan of using generic constructors, such as - and {},
to encode specific semantics; that's why I have been replacing bools
in the compiler with purpose-specific types for a while now.
I would prefer using terms such as

   module_allowed_deps(foo, [x, y])
   module_nonallowed_deps(foo, [bar, baz])

In fact, we could allow the deps spec file to contain any entry
for a module using either (but obviously not both) of these two forms.

I would also change the overall deps file syntax from

   { entry, entry, ... }

where each entry is one of the above, to just

   entry.
   entry.
   ...

This would eliminate the outer {}s which have no real purpose,
and allow all entries to be written out without having to keep track
of whether the entry being written out is the last one (since the last gets
a close brace put after them, not a comma).

This would also get rid of the "something after the one term we expect"
error type, replacing it with the already existing "malformed entry" error type.

> This is of course similar to my pragma no_trans_opt proposal,
> only contained in a single file (arguably better).

I think it is better. And supporting the specification of both allowed
and nonallowed deps satisfies both your preference and mine.

Zoltan.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: make-trans-opt-deps-spec.review
Type: application/octet-stream
Size: 9127 bytes
Desc: not available
URL: <http://lists.mercurylang.org/archives/reviews/attachments/20221219/67692af0/attachment.obj>