[m-rev.] for review: fix bug 311

[Zoltan Somogyi]

On Tue, 29 Jul 2014 10:57:50 +0200 (CEST), "Zoltan Somogyi" <zoltan.somogyi at runbox.com> wrote:
> > > @@ -621,7 +619,9 @@ common_optimise_call_2(SeenCall, InputArgs, OutputArgs, Modes, GoalInfo,
> > >              simplify_info_get_module_info(!.Info, ModuleInfo),
> > >              modes_to_uni_modes(ModuleInfo, Modes, Modes, UniModes),
> > >              create_output_unifications(GoalInfo, OutputArgs, OutputArgs2,
> > > -                UniModes, Goals, !Info),
> > > +                UniModes, Goals, Common0, _Common1, !Info),
> > > +            % XXX should be Common = Common1
> > > +            Common = Common0,
> > 
> > If that must be left as an XXX, make the comment say why it is not currently
> > Common1.
> 
> I don't know why it isn't; I was just following the logic of the existing code.

It turns out does not much matter, because:

- create_output_unifications returns a list of goals, which we always
  turned into a conjunction (even if it had only one goal),
- the call that this optimises is virtually always inside a conjunction,
  so simplify_conj is this code's ancestor. When simplify_conj saw
  that a returned goal was itself a conjunction, it processed it AGAIN.
  The second time it processed it, it took away from it a data
  structure equivalent to Common1.

I changed this to return Common1 directly, and to wrap the
assignment unifications returned bny create_output_unifications
in a conjunction only if it is not a singleton. This should generate
the same code as before, but faster.

> 
> > >              % Delete unreachable goals.
> > >              (
> > > -                simplify_info_get_instmap(!.Info, InstMap1),
> > > -                instmap_is_unreachable(InstMap1)
> > > +                % XXX InstMap1 after
> > > +                % update_instmap(Goal1, InstMap0, InstMap1),
> > > +                instmap_is_unreachable(InstMap0)
> > 
> > Is that XXX something you plan to commit or just a note to yourself?
> > (If the former, then more detail is required).

It turns out this does not make much difference either. It is mode
analysis that creates the instmap deltas seen by the first invocation
of simplification. But when mode analysis creates an instmap delta
that yields an unreachable delta, it also deletes the following conjuncts.
So if this test for an unreachable instmap is triggered, the work of
the following code has already been done.

I kept the code in (the InstMap1 version I suggested above)
in case some transformation after the front end creates
unreachable code without deleting it itself. Not likely, but
it can happen.

Zoltan.