[m-rev.] for review: set optimization options just once

[Zoltan Somogyi]

2020-10-11 15:45 GMT+11:00 "Julien Fischer" <jfischer at opturion.com>:
> On Sun, 11 Oct 2020, Zoltan Somogyi wrote:
> 
>> For review by Julien, since it is an attempt to fix a bug he reported,
>> Mantis 522. The optimization improvement mentioned in the log message
>> has bootchecked, but I haven't tried it out on this code yet.
> 
> I will try it out after you commit.

I am pretty sure that this diff won't fix 522 by itself, but it will do so
*after* a diff I will post later today is not just committed but also installed.
So there is no point in your trying it tonight; try it tomorrow.

>>          (
>> -            LotsOfHOSpec = yes,
>> -            !OptTuple ^ ot_opt_higher_order := opt_higher_order,
>> -            !OptTuple ^ ot_higher_order_size_limit := 999999
>> +            LotsOfHOSpec = bool.yes,
>> +            % XXX We used to switch on --optimize-higher-order here,
>> +            % even if it was disabled e.g. for execution tracing.
>> +            % That seems wrong.
> 
> It was probably wrong but nobody noticed.  The main use case for lots of ho
> specialization was deep profiling which is rarely used in conjunction with
> execution tracing.

Agreed. I have modified the comment to say so.

>>      globals.lookup_bool_option(!.Globals, reorder_conj, ReorderConj),
>>      (
>> -        ReorderConj = no,
>> -        !OptTuple ^ ot_deforest := do_not_deforest,
>> -        !OptTuple ^ ot_prop_constraints := do_not_prop_constraints,
>> -        !OptTuple ^ ot_prop_local_constraints := do_not_prop_local_constraints
>> +        ReorderConj = bool.no,
>> +        AllowDeforestReorderConj = bool.no,
>> +        % YYY AllowPropConstraintsReorderConj = bool.no,
>> +        AllowPropLocalConstraintsReorderConj = bool.no
>>      ;
>> -        ReorderConj = yes
>> +        ReorderConj = bool.yes,
>> +        AllowDeforestReorderConj = bool.yes,
>> +        % YYY AllowPropConstraintsReorderConj = bool.yes
>> +        AllowPropLocalConstraintsReorderConj = bool.yes
>>      ),
> 
> What's with the YYY comments?

I updated the handling of optimization options one-by-one, and happened
to do ot_prop_constraints before ot_prop_local_constraints. The % YYY lines
record conditions under which ot_prop_constraints should be disabled,
before I noticed that the AllowPropLocalConstraints* vars looked like
they could do the job for *both* options. I kept them around commented out
in case this insight turned out to be false. Since it hasn't, I am deleting the
commented-out lines.

Zoltan.