oracle / graal

GraalVM compiles Java applications into native executables that start instantly, scale fast, and use fewer compute resources 🚀
https://www.graalvm.org
Other
20.36k stars 1.64k forks source link

Remainder nodes are always fixed, even when they can't divide-by-zero #3866

Closed chrisseaton closed 2 years ago

chrisseaton commented 3 years ago

For example this Ruby code:

def foo(x)
  x.times do |n|
    if (n % 5 == 0) && (n % 3 == 0)
      out :fizzbuzz
    elsif n % 5 == 0
      out :fizz
    elsif n % 3 == 0
      out :buzz
    else
      out n
    end
  end
end

I'd expect n % 5 == 0 and n % 3 == 0 to be calculated once each, but they aren't, as the nodes are fixed and so GVN fails. They needn't be fixed, as they can't divide-by-zero.

graph

@davleopo

davleopo commented 3 years ago

@chrisseaton thanks for the report, Ill have a look.

davleopo commented 2 years ago

https://github.com/oracle/graal/commit/39bbdedcaefbccb9369fbf2dab4cec5e88b446c9 adds support to float integer division nodes early on in the compilation pipeline if we know they can never trap. Additionally, we float all fixed div nodes during lowering under certain conditions and can gvn them as well if they have equal, non-constant inputs. Late in the compilation pipeline we can also fold them together again to remove the explicit check if we can perform the div by zero check implicitly.

chrisseaton commented 2 years ago

Wow that's a huge commit to fix this. Thanks very much!