Skip to content

sys_core_fold: Eliminate crash in beam_kernel_to_ssa, again #6624

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 1 commit into from
Jan 4, 2023
Merged

sys_core_fold: Eliminate crash in beam_kernel_to_ssa, again #6624

merged 1 commit into from
Jan 4, 2023

Conversation

jhogberg
Copy link
Contributor

@jhogberg jhogberg commented Jan 3, 2023

Fixes #6612

@jhogberg jhogberg added team:VM Assigned to OTP team VM testing currently being tested, tag is used by OTP internal CI labels Jan 3, 2023
@jhogberg jhogberg self-assigned this Jan 3, 2023
@github-actions
Copy link
Contributor

github-actions bot commented Jan 3, 2023 •
edited
Loading

CT Test Results

       2 files     285 suites   11m 2s ⏱️
   751 tests    749 ✔️ 2 💤 0 ❌
4 655 runs  4 653 ✔️ 2 💤 0 ❌

Results for commit 5076069.

♻️ This comment has been updated with latest results.

To speed up review, make sure that you have read Contributing to Erlang/OTP and that all checks pass.

See the TESTING and DEVELOPMENT HowTo guides for details about how to run test locally.

Artifacts

// Erlang/OTP Github Action Bot

@jhogberg jhogberg merged commit eaf1a6f into erlang:master Jan 4, 2023
@RobinMorisset RobinMorisset mentioned this pull request Jan 4, 2023
Closed
bjorng added a commit to bjorng/otp that referenced this pull request Jan 9, 2023
@bjorng
sys_core_fold: Remove problematic optimization
00aa586 
The optimization of nested lets was unsafe if the number
of iterations for `sys_core_fold` ran out. There have been
several attempts to fix that problem:

5076069 (erlang#6624)
30d4c45 (erlang#6631)
ff05bee (erlang#6575)

It's time to cut the Gordian knot by removing the optimization.
While at it, remove all optimizations of nested lets, not only
the offending part of the optimization.

Running `scripts/diffable` before and after and diffing, shows changes
in 40 modules. Most of them are neutral (the same instructions in a
different order). There are few changes where an extra `test_heap`
instruction has been introduced, for example in the `compile` module:

    -    {test_heap,19,1}.
    +    {test_heap,15,1}.
         {update_record,{atom,reuse},14,{y,0},{x,0},{list,[7,{x,0}]}}.
    -    {put_tuple2,{x,0},{list,[{atom,ok},{y,1},{x,0}]}}.
         {try_end,{y,2}}.
    +    {test_heap,4,1}.
    +    {put_tuple2,{x,0},{list,[{atom,ok},{y,1},{x,0}]}}.
         {deallocate,3}.
         return.

which was compiled from this source code:

    try
        Mod = cerl:concrete(cerl:module_name(Core)),
        {ok,Core,St#compile{module=Mod}}
    catch
        _:_ ->
            {ok,Core,St}
    end.

In the `ct_run` module, the following code:

    case proplists:get_value(step, Misc) of
        undefined ->
            #opts{};
        StepOpts ->
            #opts{step = StepOpts}
    end

is no longer optimized to:

    StepOpts = proplists:get_value(step, Misc)
    #opts{step = StepOpts}

There are also improvements in a few modules.

Closes erlang#6633
Closes erlang#6635
@bjorng bjorng mentioned this pull request Jan 9, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@jhogberg jhogberg

Labels
team:VM Assigned to OTP team VM testing currently being tested, tag is used by OTP internal CI
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[erlc] internal error in beam_kernel_to_ssa:collect_preds
1 participant
@jhogberg