[m-rev.] for review: fix simplification of multi-arm switch

[Peter Wang]

Branches: main

Fix a simplification bug which was introduced with multi-cons_id switch arms.
Previously a singleton switch could be replaced by the case goal, possibly
with a functor test beforehand, but that's only true if the case arm is
applicable to only a single functor.

compiler/simplify.m:
        As above.

tests/hard_coded/Mercury.options:
tests/hard_coded/Mmakefile:
tests/hard_coded/simplify_multi_arm_switch.exp:
tests/hard_coded/simplify_multi_arm_switch.m:
        Add test case.

diff --git a/compiler/simplify.m b/compiler/simplify.m
index 7c101fa..47eeedd 100644
--- a/compiler/simplify.m
+++ b/compiler/simplify.m
@@ -1071,19 +1071,21 @@ simplify_goal_switch(GoalExpr0, GoalExpr,
GoalInfo0, GoalInfo, !Info) :-
         SwitchCanFail0, SwitchCanFail, !.Info, !Info),
     list.reverse(RevCases, Cases),
     (
-        Cases = [],
+        Cases = []
+    ->
         % An empty switch always fails.
         simplify_info_incr_cost_delta(cost_of_eliminate_switch, !Info),
         Context = goal_info_get_context(GoalInfo0),
         hlds_goal(GoalExpr, GoalInfo) = fail_goal_with_context(Context)
     ;
         Cases = [case(MainConsId, OtherConsIds, SingleGoal)],
+        OtherConsIds = []
+    ->
         % A singleton switch is equivalent to the goal itself with a
         % possibly can_fail unification with the functor on the front.
         MainConsIdArity = cons_id_arity(MainConsId),
         (
             SwitchCanFail = can_fail,
-            OtherConsIds = [],
             MaybeConsIds \= yes([MainConsId])
         ->
             % Don't optimize in the case of an existentially typed constructor
@@ -1135,7 +1137,6 @@ simplify_goal_switch(GoalExpr0, GoalExpr,
GoalInfo0, GoalInfo, !Info) :-
         ),
         simplify_info_incr_cost_delta(cost_of_eliminate_switch, !Info)
     ;
-        Cases = [_, _ | _],
         GoalExpr = switch(Var, SwitchCanFail, Cases),
         (
             ( goal_info_has_feature(GoalInfo0, feature_mode_check_clauses_goal)
diff --git a/tests/hard_coded/Mercury.options b/tests/hard_coded/Mercury.options
index a2cd8e7..91c64dd 100644
--- a/tests/hard_coded/Mercury.options
+++ b/tests/hard_coded/Mercury.options
@@ -42,6 +42,7 @@ MCFLAGS-intermod_multimode_main = --intermodule-optimization
 MCFLAGS-lco_no_inline	    =	--optimize-constructor-last-call
--no-inline-builtins
 MCFLAGS-reuse_ho            =	--ctgc --no-optimise-higher-order
 MCFLAGS-sharing_comb	    =	--ctgc --structure-sharing-widening 2
+MCFLAGS-simplify_multi_arm_switch = -O3
 MCFLAGS-uncond_reuse	    =	--ctgc
 MCFLAGS-uncond_reuse_bad    =	--ctgc

diff --git a/tests/hard_coded/Mmakefile b/tests/hard_coded/Mmakefile
index 6271558..5c2dbd4 100644
--- a/tests/hard_coded/Mmakefile
+++ b/tests/hard_coded/Mmakefile
@@ -206,6 +206,7 @@ ORDINARY_PROGS=	\
 	semi_disj \
 	setjmp_test \
 	shift_test \
+	simplify_multi_arm_switch \
 	solve_quadratic \
 	space \
 	stable_sort \
diff --git a/tests/hard_coded/simplify_multi_arm_switch.exp
b/tests/hard_coded/simplify_multi_arm_switch.exp
new file mode 100644
index 0000000..9766475
--- /dev/null
+++ b/tests/hard_coded/simplify_multi_arm_switch.exp
@@ -0,0 +1 @@
+ok
diff --git a/tests/hard_coded/simplify_multi_arm_switch.m
b/tests/hard_coded/simplify_multi_arm_switch.m
new file mode 100644
index 0000000..cbdeaa4
--- /dev/null
+++ b/tests/hard_coded/simplify_multi_arm_switch.m
@@ -0,0 +1,51 @@
+% Regression test.
+% Previously a singleton switch could be replaced by the case goal, possibly
+% with a functor test beforehand, but that's only true if the case arm is
+% applicable to only a single functor.
+
+:- module simplify_multi_arm_switch.
+:- interface.
+
+:- import_module io.
+
+:- pred main(io::di, io::uo) is det.
+
+%-----------------------------------------------------------------------------%
+%-----------------------------------------------------------------------------%
+
+:- implementation.
+
+:- import_module bool.
+
+:- type fruit
+    --->    apple
+    ;       pear
+    ;       banana
+    ;       plum.
+
+:- pred squishy(fruit::in) is semidet.
+:- pragma no_inline(squishy/1).
+
+squishy(Fruit) :-
+    (
+        ( Fruit = apple
+        ; Fruit = pear
+        ),
+        IsSquishy = no
+    ;
+        ( Fruit = banana
+        ; Fruit = plum
+        ),
+        IsSquishy = yes
+    ),
+    IsSquishy = yes.
+
+main(!IO) :-
+    ( squishy(apple) ->
+        io.write_string("bug!\n", !IO)
+    ;
+        io.write_string("ok\n", !IO)
+    ).
+
+%-----------------------------------------------------------------------------%
+% vim: ft=mercury ts=8 sts=4 sw=4 et
--------------------------------------------------------------------------
mercury-reviews mailing list
Post messages to:       mercury-reviews at csse.unimelb.edu.au
Administrative Queries: owner-mercury-reviews at csse.unimelb.edu.au
Subscriptions:          mercury-reviews-request at csse.unimelb.edu.au
--------------------------------------------------------------------------