[m-rev.] for post-commit review: simplify management of smart indexing

[Julien Fischer]

On Mon, 3 Apr 2023, Zoltan Somogyi wrote:

> This diff was tickled by a reference to switch_gen.m in a post
> on reddit.com/r/Compilers, of all things.

That doesn't seem an entirely unreasonable for it to be referred to.

> Next, I plan to simplify the logic of ml_gen_smart_string_switch,
> whose complexity may have been needed when we had IL
> and gcc asm as targets, but isn't needed now. Unless someone objects,
> this simplification will include the deletion of the test for "does this
> MLDS target support switches on ints?", since the only target for which
> this was "no" was IL, and I don't expect we will directly target
> any language that low level ever again. (The smart way to go
> when targeting a *normal* assembler would be to generate
> the same MLDS as we would for C, and then *compiling* that MLDS
> into assembler, instead of emitting it as C.)

No objections from me.

> Simplify the management of smart indexing.
> 
> compiler/switch_util.m:
>     Simplify the interface of the find_switch_category predicate.
>     Instead of returning (a) a *smart switch category*, and (b) a flag
>     that says whether that category may be used, simply return
>     a *switch category*, which includes a non-smart switch category,
>     the if-then-else chain.
>
>     Give some data constructors in the switch_category type names that
>     clarify the differences between them.
>
>     Move the type_ctor_cat_to_switch_cat function just after its only caller.
> 
> compiler/ml_switch_gen.m:
> compiler/switch_gen.m:
>     Flatten the switches on the return values of find_switch_category.
>
>     Merge arms of these switches that handle dumb switches and switches
>     on floats, because they are both implemented as if-then-else chains.
>
>     In ml_switch_gen.m, inline the ml_gen_smart_int64_switch predicate,
>     which made only one decision, at its only call site.
>
>     Add XXXs that point out opportunities for further simplification.

...


> diff --git a/compiler/switch_util.m b/compiler/switch_util.m
> index 722c0445a..8b50de279 100644
> --- a/compiler/switch_util.m
> +++ b/compiler/switch_util.m

...

> @@ -111,15 +94,35 @@
>      %
>  :- func estimate_switch_tag_test_cost(cons_tag) = int.
> 
> -:- type may_use_smart_indexing
> -    --->    may_not_use_smart_indexing
> -    ;       may_use_smart_indexing.
> +:- type switch_category
> +    --->    ite_chain_switch
> +            % A chain of if-then-elses; a dumb switch.
> +
> +    ;       int_max_32_switch
> +            % A switch on int, uint, char, enum, or intN/uintN where N < 64.
> +
> +    ;       int_64_switch
> +            % A switch on a int64 or uint64.
> +            % These require special treatment on the Java backend.

I suggest:

      These require special treatment on the Java backend because Java
      does not support switches on longs.

Otherwise, that's fine.

Julien.