[m-rev.] for post-commit review: fix github issue 133

Peter Wang novalazy at gmail.com
Fri Jul 12 14:59:51 AEST 2024


On Thu, 11 Jul 2024 19:06:59 +1000 "Zoltan Somogyi" <zoltan.somogyi at runbox.com> wrote:
> Fix compiler abort for undeleted dead code.
> 
> This fixes Github issue #133.

> diff --git a/compiler/jumpopt.m b/compiler/jumpopt.m
> index bea8771da..4b9acace4 100644
> --- a/compiler/jumpopt.m
> +++ b/compiler/jumpopt.m
> @@ -846,16 +846,17 @@ jump_opt_if_val(Uinstr0, Comment0, Instrs0, _PrevInstr, JumpOptInfo,
>                  % Attempt to transform code such as
>                  %
>                  %       if (Cond) L1
> -                %       r1 = MR_TRUE
> +                %       r1 = MR_FALSE           (or MR_TRUE)
> +                % when followed by
>                  %       <epilog>
>                  %       ...
>                  %     L1:
> -                %       r1 = MR_FALSE
> +                %       r1 = MR_TRUE            (or MR_FALSE)
>                  %       <epilog>
>                  %
>                  % into
>                  %
> -                %       r1 = Cond
> +                %       r1 = Cond               (or r1 = !Cond)
>                  %       <epilog>
>                  %
>                  opt_util.is_sdproceed_next(Instrs0, BetweenFT),

I saw an old workaround around here that we might want to remove:

                not needs_workaround(reg(reg_r, 1), NewCond)

> @@ -882,7 +883,8 @@ jump_opt_if_val(Uinstr0, Comment0, Instrs0, _PrevInstr, JumpOptInfo,
>                  ),
>                  Proceed = llds_instr(goto(code_succip), "shortcircuit"),
>                  NewInstrs = [NewAssign | Between] ++ [Proceed],
> -                NewRemain = nr_specified(NewInstrs, Instrs0)
> +                skip_to_next_label(Instrs0, _DeadInstrs, LiveInstrs0),
> +                NewRemain = nr_specified(NewInstrs, LiveInstrs0)
>              else if
>                  % Try to short-circuit the destination.
>                  TargetLabel \= DestLabel

The diff seems fine (I'm not familiar with the code).

Peter


More information about the reviews mailing list