Skip to content
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.

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

setelement optimization turns to a pessimization in specific scenarios #9198

Open
michalmuskala opened this issue Dec 16, 2024 · 1 comment
Assignees
Labels
bug Issue is reported as a bug team:VM Assigned to OTP team VM

Comments

@michalmuskala
Copy link
Contributor

michalmuskala commented Dec 16, 2024

Describe the bug
Given the following code:

-module(test).

-export([test/1]).

test(T) when tuple_size(T) > 10 ->
    T1 = setelement(1, T, a),
    setelement(2, T1, b).

The compiler produces following bytecode (when compiled with erlc +to_asm test.erl):

    {bif,tuple_size,{f,1},[{x,0}],{x,1}}.
    {test,is_ge,{f,1},[{tr,{x,1},{t_integer,{0,16777215}}},{integer,11}]}.
    {allocate,1,1}.
    {move,{x,0},{y,0}}.
    {move,{x,0},{x,1}}.
    {move,{atom,a},{x,2}}.
    {move,{integer,1},{x,0}}.
    {line,[{location,"test.erl",6}]}.
    {call_ext,3,{extfunc,erlang,setelement,3}}.
    {move,{y,0},{x,1}}.
    {move,{atom,b},{x,2}}.
    {trim,1,0}.
    {move,{integer,2},{x,0}}.
    {line,[{location,"test.erl",7}]}.
    {call_ext,3,{extfunc,erlang,setelement,3}}.
    {set_tuple_element,{atom,a},{x,0},0}.
    {deallocate,0}.
    return.

Notice that the field 1 is updated twice to the atom a - first in the first setelement call (which result is completely thrown away) and then again after the second call to setelement.

With optimisations disabled and compiling with erlc +to_asm +no_ssa_opt test.erl we end up with:

    {bif,tuple_size,{f,1},[{x,0}],{x,1}}.
    {test,is_lt,{f,1},[{integer,10},{x,1}]}.
    {allocate,0,1}.
    {move,{x,0},{x,1}}.
    {move,{atom,a},{x,2}}.
    {move,{integer,1},{x,0}}.
    {line,[{location,"test.erl",6}]}.
    {call_ext,3,{extfunc,erlang,setelement,3}}.
    {move,{x,0},{x,1}}.
    {move,{atom,b},{x,2}}.
    {move,{integer,2},{x,0}}.
    {line,[{location,"test.erl",7}]}.
    {call_ext_last,3,{extfunc,erlang,setelement,3},0}.

which is actually better - no stack juggling and no unnecessary operation.

Expected behavior
I would expect the produced code to be something like:

    {bif,tuple_size,{f,1},[{x,0}],{x,1}}.
    {test,is_lt,{f,1},[{integer,10},{x,1}]}.
    {allocate,0,1}.
    {move,{x,0},{x,1}}.
    {move,{atom,b},{x,2}}.
    {move,{integer,2},{x,0}}.
    {line,[{location,"test.erl",7}]}.
    {call_ext,3,{extfunc,erlang,setelement,3}}.
    {set_tuple_element,{atom,a},{x,0},0}.
    {deallocate,0}.
    return.

Taking advantage of the set_tuple_element optimisation and avoiding the unnecessary extra call to setelement. This is possible because of the guard, we know the first setelement operation can't fail.

I believe what happens is that the set_tuple_element optimisation expects another pass to remove the, now redundant, first call to setelement, but for some reason that is not happening correctly.

Affected versions
Tested on OTP 27.1

@michalmuskala michalmuskala added the bug Issue is reported as a bug label Dec 16, 2024
@jhogberg jhogberg self-assigned this Dec 16, 2024
@jhogberg jhogberg added the team:VM Assigned to OTP team VM label Dec 16, 2024
@jhogberg
Copy link
Contributor

Thanks for your report, the compiler doesn't forward tuple_size(T) > 10 into the type of T. It would be trivial to fix if not for #7342.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Issue is reported as a bug team:VM Assigned to OTP team VM
Projects
None yet
Development

No branches or pull requests

2 participants