eclipse / omr

Eclipse OMR™ Cross platform components for building reliable, high performance language runtimes
http://www.eclipse.org/omr
Other
933 stars 392 forks source link

Replace redundant shifts and rotations of const 0 with const 0 #7364

Closed dylanjtuttle closed 1 week ago

dylanjtuttle commented 3 weeks ago

Since applying a shift or rotation to 0 simply produces 0 again, the optimizer can replace any shift or rotation for which the first child is a const 0 with a const 0.

Issue: openj9 #19576

dylanjtuttle commented 3 weeks ago

As a first draft, I tried my best to use other examples across the file to figure out what to do (and to apply the same logic to all of the shift and rotation simplifiers). I still need to run the test in which @knn-k noticed the issue to see if this resolves it.

knn-k commented 3 weeks ago

Thank you for opening this PR. I ran the testcase MathLoadTest_bigdecimal_CS_5m with it, and I see the ishl nodes for arraycopy calls are replaced as expected.

Before simplification:

n14517n     call  java/lang/System.arraycopy(Ljava/lang/Object;ILjava/lang/Object;II)V[#2155  final native static Method] [flags 0x20500 0x0 ] ()  [       0x13883a960] bci=[118,27,5251] rc=1 vc=3134 vn=- li=-1 udi=- 
nc=5 flg=0x20
n14506n       aloadi  java/lang/String.value [B[#1134  final Shadow +8] [flags 0x400a0607 0x0 ]  [       0x13883a5f0] bci=[118,10,5251] rc=1 vc=3134 vn=- li=-1 udi=- nc=1
n14505n         aload  <temp slot 5>[#2121  Auto] [flags 0x7 0x0 ] (X!=0 )                    [       0x13883a5a0] bci=[118,9,5251] rc=1 vc=3134 vn=- li=-1 udi=- nc=0 flg=0x4
n14509n       ishl                                                                            [       0x13883a6e0] bci=[118,16,5251] rc=1 vc=3134 vn=- li=-1 udi=- nc=2
n14654n         iconst 0 (X==0 X>=0 X<=0 )                                                    [       0x13883d430] bci=[112,3,1724] rc=1 vc=3134 vn=- li=-1 udi=- nc=0 flg=0x302
n14508n         iload  <temp slot 9>[#2167  Auto] [flags 0x20000003 0x0 ]                     [       0x13883a690] bci=[118,14,5251] rc=1 vc=3134 vn=- li=-1 udi=- nc=0
n14510n       aload  <temp slot 8>[#2166  Auto] [flags 0x20000007 0x0 ]                       [       0x13883a730] bci=[118,17,5251] rc=1 vc=3134 vn=- li=-1 udi=- nc=0
n14513n       ishl                                                                            [       0x13883a820] bci=[118,21,5251] rc=1 vc=3134 vn=- li=-1 udi=- nc=2
n14511n         iload  <temp slot 6>[#2122  Auto] [flags 0x3 0x0 ]                            [       0x13883a780] bci=[118,18,5251] rc=1 vc=3134 vn=- li=-1 udi=- nc=0
n14512n         iload  <temp slot 9>[#2167  Auto] [flags 0x20000003 0x0 ]                     [       0x13883a7d0] bci=[118,19,5251] rc=1 vc=3134 vn=- li=-1 udi=- nc=0
n14516n       ishl                                                                            [       0x13883a910] bci=[118,26,5251] rc=1 vc=3134 vn=- li=-1 udi=- nc=2
n14514n         iload  <temp slot 10>[#2168  Auto] [flags 0x20000003 0x0 ]                    [       0x13883a870] bci=[118,22,5251] rc=1 vc=3134 vn=- li=-1 udi=- nc=0
n14515n         iload  <temp slot 9>[#2167  Auto] [flags 0x20000003 0x0 ]                     [       0x13883a8c0] bci=[118,24,5251] rc=1 vc=3134 vn=- li=-1 udi=- nc=0

After simplification:

[  8516] O^O TREE SIMPLIFICATION: Removed redundant ishl node [       0x13883a6e0]

n14517n     call  java/lang/System.arraycopy(Ljava/lang/Object;ILjava/lang/Object;II)V[#2155  final native static Method] [flags 0x20500 0x0 ] ()  [       0x13883a960] bci=[118,27,5251] rc=1 vc=4024 vn=- li=- udi=- nc=5 flg=0x20
n14506n       aloadi  java/lang/String.value [B[#1134  final Shadow +8] [flags 0x400a0607 0x0 ]  [       0x13883a5f0] bci=[118,10,5251] rc=1 vc=4024 vn=- li=- udi=- nc=1
n14505n         aload  <temp slot 5>[#2121  Auto] [flags 0x7 0x0 ] (X!=0 X>=0 )               [       0x13883a5a0] bci=[118,9,5251] rc=1 vc=4024 vn=- li=- udi=- nc=0 flg=0x104
n14654n       iconst 0 (X==0 X>=0 X<=0 )                                                      [       0x13883d430] bci=[112,3,1724] rc=1 vc=4024 vn=- li=- udi=- nc=0 flg=0x302
n14510n       aload  <temp slot 8>[#2166  Auto] [flags 0x20000007 0x0 ]                       [       0x13883a730] bci=[118,17,5251] rc=1 vc=4024 vn=- li=- udi=- nc=0
n14513n       ishl                                                                            [       0x13883a820] bci=[118,21,5251] rc=1 vc=4024 vn=- li=- udi=- nc=2
n14511n         iload  <temp slot 6>[#2122  Auto] [flags 0x3 0x0 ]                            [       0x13883a780] bci=[118,18,5251] rc=1 vc=4024 vn=- li=- udi=- nc=0
n14512n         iload  <temp slot 9>[#2167  Auto] [flags 0x20000003 0x0 ]                     [       0x13883a7d0] bci=[118,19,5251] rc=1 vc=4024 vn=- li=- udi=- nc=0
n14516n       ishl                                                                            [       0x13883a910] bci=[118,26,5251] rc=1 vc=4024 vn=- li=- udi=- nc=2
n14514n         iload  <temp slot 10>[#2168  Auto] [flags 0x20000003 0x0 ]                    [       0x13883a870] bci=[118,22,5251] rc=1 vc=4024 vn=- li=- udi=- nc=0
n14515n         iload  <temp slot 9>[#2167  Auto] [flags 0x20000003 0x0 ]                     [       0x13883a8c0] bci=[118,24,5251] rc=1 vc=4024 vn=- li=- udi=- nc=0
dylanjtuttle commented 3 weeks ago

Thank you for running that test for me, @knn-k! @hzongaro, would you mind giving this a review when you get a chance?

dylanjtuttle commented 2 weeks ago

@hzongaro Oh I see what you mean, I'll work on addressing that in this PR as well!

hzongaro commented 1 week ago

Jenkins test sanity.functional all jdk8,jdk17,jdk21

knn-k commented 1 week ago

Jenkins build all

This is a PR in OMR. You cannot run OpenJ9 sanity.functional tests here.

hzongaro commented 1 week ago

This is a PR in OMR. You cannot run OpenJ9 sanity.functional tests here.

Sigh! Sorry for my confusion. I have too many tabs open.

hzongaro commented 1 week ago

Testing was successful. Merging.