Skip to content

Track distinct live commoned loads of locals for Store Sinking - #7844

Merged
vijaysun-omr merged 2 commits into
eclipse-omr:masterfrom
hzongaro:issue17033-commoning-store-sinking
Jul 23, 2025
Merged

Track distinct live commoned loads of locals for Store Sinking#7844
vijaysun-omr merged 2 commits into
eclipse-omr:masterfrom
hzongaro:issue17033-commoning-store-sinking

Conversation

@hzongaro

Copy link
Copy Markdown
Contributor

Store sinking tracks which local variables are used in the value assigned to a candidate store, tracks whether a load of a local variable has been commoned, and whether the load might be killed by a store to that local. All this tracking is done on the basis of the index of the local variable.

However, it's possible for a local to be loaded before a store to that local, and the value that existed before that store to be used by the candidate for store sinking. In general, there could be several loads and stores to such a local interleaved, with each of the distinct values loaded being used by commoned references in the store that's a candidate for store sinking.

This problem was fixed by having LiveVariableInformation keep track of the distinct nodes that load local variables in backwards tree walks, and the number of distinct loads for each local variable.

Also removed a redundant copy into *savedLiveCommonedLoads of the result of a call to TR_LiveVariableInformation::liveCommonedLoads(), and removed an unused TR_BitVector satisfiedLiveCommonedLoads.

I will add an illustrative example below.

Fixes: eclipse-openj9/openj9#17033

Store sinking tracks which local variables are used in the value
assigned to a candidate store, tracks whether a load of a local variable
has been commoned, and whether the load might be killed by a store to
that local.  All this tracking is done on the basis of the index of the
local variable.

However, it's possible for a local to be loaded before a store to that
local, and the value that existed before that store to be used by the
candidate for store sinking.  In general, there could be several loads
and stores to such a local interleaved, with each of the distinct values
loaded being used by commoned references in the store that's a candidate
for store sinking.

This problem was fixed by having LiveVariableInformation keep track of
the distinct nodes that load local variables in backwards tree walks,
and the number of distinct loads for each local variable.

Also removed a redundant copy into *savedLiveCommonedLoads of the result
of a call to TR_LiveVariableInformation::liveCommonedLoads(), and
removed an unused TR_BitVector satisfiedLiveCommonedLoads.

Signed-off-by:  Henry Zongaro <zongaro@ca.ibm.com>
@hzongaro

Copy link
Copy Markdown
Contributor Author

The following example is taken from eclipse-openj9/openj9#22145.

Trees before General Store Sinking and snippets from trace:
n409n     BBStart <block_28> (freq 2) (cold) (in loop 5)                                      [0x7f74744bdf90] bci=[-1,18,12] rc=0 vc=1679 vn=- li=- udi=- nc=0
n236n     treetop                                                                             [0x7f74744ba980] bci=[-1,106,27] rc=0 vc=1679 vn=- li=- udi=- nc=1
n230n       iload  <auto slot 0>[#419  Auto] [flags 0x3 0x0 ] (cannotOverflow )               [0x7f74744ba7a0] bci=[-1,104,27] rc=2 vc=1679 vn=- li=- udi=35 nc=0 flg=0x1000
n237n     treetop                                                                             [0x7f74744ba9d0] bci=[-1,106,27] rc=0 vc=1679 vn=- li=- udi=- nc=1
n231n       iload  <auto slot 0>[#419  Auto] [flags 0x3 0x0 ] (cannotOverflow )               [0x7f74744ba7f0] bci=[-1,105,27] rc=2 vc=1679 vn=- li=- udi=36 nc=0 flg=0x1000
n235n     istore  <auto slot 0>[#419  Auto] [flags 0x3 0x0 ]                                  [0x7f74744ba930] bci=[-1,106,27] rc=0 vc=1679 vn=- li=25 udi=13 nc=1
n234n       isub                                                                              [0x7f74744ba8e0] bci=[-1,106,27] rc=1 vc=1679 vn=- li=- udi=- nc=2
n232n         iload  <auto slot 0>[#419  Auto] [flags 0x3 0x0 ] (cannotOverflow )             [0x7f74744ba840] bci=[-1,106,27] rc=1 vc=1679 vn=- li=- udi=37 nc=0 flg=0x1000
n233n         iconst -1 (X!=0 X<=0 )                                                          [0x7f74744ba890] bci=[-1,106,27] rc=1 vc=1679 vn=- li=- udi=- nc=0 flg=0x204
n239n     istore  <auto slot 2>[#421  Auto] [flags 0x3 0x0 ]                                  [0x7f74744baa70] bci=[-1,110,27] rc=0 vc=1679 vn=- li=26 udi=14 nc=1
n238n       iadd                                                                              [0x7f74744baa20] bci=[-1,109,27] rc=1 vc=1679 vn=- li=- udi=- nc=2
n230n         ==>iload
n231n         ==>iload
...
n368n     BBEnd </block_28> (cold)                                                            [0x7f74744bd2c0] bci=[-1,18,12] rc=0 vc=1679 vn=- li=- udi=- nc=0
n326n     BBStart <block_15> (freq 2) (extension of previous block) (cold) (in loop 3)        [0x7f74744bc5a0] bci=[-1,22,12] rc=0 vc=1679 vn=- li=- udi=- nc=0
n325n     goto --> block_8 BBStart at n1n                                                     [0x7f74744bc550] bci=[-1,22,12] rc=0 vc=1679 vn=- li=- udi=- nc=0
n327n     BBEnd </block_15> (cold) =====                                                      [0x7f74744bc5f0] bci=[-1,22,12] rc=0 vc=1679 vn=- li=- udi=- nc=0

...

#419:  <auto slot 0>[ Auto] [flags 0x3 0x0 ] [0x7f7474487ce0] (Int32)
...
Local #23 is symbol at 00007F7474487CE0
...
  Looking for stores to sink in block_28
  ...
      Examining node [00007F74744BAA70] in block_28  [n239n]
      savedLiveCommonedLoads: {}
      calling findLocalUses on node 00007F74744BAA70 with treeVisitCount 1708
         Looking backward for uses in node 00007F74744BAA70 has future use count = 0 and reference count = 0
            first reference
         Looking backward for uses in node 00007F74744BAA20 has future use count = 1 and reference count = 1
            first reference
         Looking backward for uses in node 00007F74744BA7A0 [n230n] has future use count = 2 and reference count = 2
            not first reference
            Node [00007F74744BA7A0] local [00007F7474487CE0] idx 23
              Marking as commoned load
              Marking 23 as live commoned load
         Looking backward for uses in node 00007F74744BA7F0 [n231n] has future use count = 2 and reference count = 2
            not first reference
            Node [00007F74744BA7F0] local [00007F7474487CE0] idx 23
              Marking as commoned load
              Marking 23 as live commoned load
      killedSymbols: {}
      usedSymbols: {}      treeCommonedLoads: {23}
      setting 21 in killedSymbols
      killedSymbols: {21}
      usedSymbols: {}
      treeCommonedLoads: {23}
      savedLiveCommonedLoads: {}
      killedLiveCommonedLoads: {}
      is store to local 21
         store passes some interfering and candidate tests
      creating use or kill info on movable node [00007F74744BAA70] to track kills and uses
      store is potentially movable, collecting commoned loads and adding to list
...
    Examining node [00007F74744BA7F0] in block_28  [n231n]
      savedLiveCommonedLoads: {}
      calling findLocalUses on node 00007F74744BA9D0 with treeVisitCount 1710
         Looking backward for uses in node 00007F74744BA9D0 has future use count = 0 and reference count = 0
            first reference
         Looking backward for uses in node 00007F74744BA7F0 has future use count = 1 and reference count = 2
            first reference
            Node [00007F74744BA7F0] local [00007F7474487CE0] idx 23
            Gening symbol with local index 23
      killedSymbols: {}
      usedSymbols: {23}      treeCommonedLoads: {}
      killedSymbols: {}
      usedSymbols: {23}
      treeCommonedLoads: {}
      savedLiveCommonedLoads: {}
      killedLiveCommonedLoads: {23}
      symbol stores to a live commoned reference, searching stores below that load this symbol via commoned reference ...
            examining store [00007F74744BABB0]
            examining store [00007F74744BAA70]
         (non-commoned) uses of killed symbols: {23}
            found store [00007F74744BAA70] below that may require a temp
               killed live commoned loads used above store:{23}
           found commonedLoad = 00007F74744BA7F0
(Transformation #0 start - create temp store)
(Transformation #0 was performed)
      creating use or kill info on non-movable node [00007F74744BA9D0] to track kills and uses
    skipping created temp store [00007F7474678590] in backward treetop walk
    Examining node [00007F74744BA7A0] in block_28
      savedLiveCommonedLoads: {}
      calling findLocalUses on node 00007F74744BA980 with treeVisitCount 1711
         Looking backward for uses in node 00007F74744BA980 has future use count = 0 and reference count = 0
            first reference
         Looking backward for uses in node 00007F74744BA7A0 has future use count = 1 and reference count = 2
            first reference
            Node [00007F74744BA7A0] local [00007F7474487CE0] idx 23
            Gening symbol with local index 23
      killedSymbols: {}
      usedSymbols: {23}      treeCommonedLoads: {}
      killedSymbols: {}
      usedSymbols: {23}
      treeCommonedLoads: {}
      savedLiveCommonedLoads: {}
      killedLiveCommonedLoads: {}
      creating use or kill info on non-movable node [00007F74744BA980] to track kills and uses
  Reached beginning of extended basic block

Notice that #419 is loaded twice by nodes n230n and n231n, and their sum is stored in #421 by node n239n. Between the two commoned loads and the store there is a store into #419 by node n235n. That's because during the backwards tree walk, when the first commoned node n231n is encountered, local symbol 23 (#419) is removed from the list of commoned symbols.

Trees following Store Sinking:
n409n     BBStart <block_28> (freq 2) (cold)                                                  [0x7f74744bdf90] bci=[-1,18,12] rc=0 vc=1689 vn=- li=- udi=- nc=0
n236n     treetop                                                                             [0x7f74744ba980] bci=[-1,106,27] rc=0 vc=1711 vn=- li=- udi=- nc=1
n230n       iload  <auto slot 0>[#419  Auto] [flags 0x3 0x0 ] (cannotOverflow )               [0x7f74744ba7a0] bci=[-1,104,27] rc=2 vc=1711 vn=- li=- udi=35 nc=0 flg=0x1000
n940n     istore  <temp slot 13>[#516  Auto] [flags 0x3 0x0 ]                                 [0x7f7474678590] bci=[-1,105,27] rc=0 vc=0 vn=- li=- udi=- nc=1
n231n       iload  <auto slot 0>[#419  Auto] [flags 0x3 0x0 ] (cannotOverflow )               [0x7f74744ba7f0] bci=[-1,105,27] rc=3 vc=1710 vn=- li=- udi=36 nc=0 flg=0x1000
n237n     treetop                                                                             [0x7f74744ba9d0] bci=[-1,106,27] rc=0 vc=1710 vn=- li=- udi=- nc=1
n231n       ==>iload
n235n     istore  <auto slot 0>[#419  Auto] [flags 0x3 0x0 ]                                  [0x7f74744ba930] bci=[-1,106,27] rc=0 vc=1709 vn=- li=- udi=13 nc=1
n234n       isub                                                                              [0x7f74744ba8e0] bci=[-1,106,27] rc=1 vc=1709 vn=- li=- udi=- nc=2
n232n         iload  <auto slot 0>[#419  Auto] [flags 0x3 0x0 ] (cannotOverflow )             [0x7f74744ba840] bci=[-1,106,27] rc=1 vc=1709 vn=- li=- udi=37 nc=0 flg=0x1000
n233n         iconst -1 (X!=0 X<=0 )                                                          [0x7f74744ba890] bci=[-1,106,27] rc=1 vc=1709 vn=- li=- udi=- nc=0 flg=0x204
n239n     treetop                                                                             [0x7f74744baa70] bci=[-1,110,27] rc=0 vc=1708 vn=- li=- udi=14 nc=1
n238n       iadd                                                                              [0x7f74744baa20] bci=[-1,109,27] rc=1 vc=1708 vn=- li=- udi=- nc=2
n230n         ==>iload
n231n         ==>iload
...
n368n     BBEnd </block_28> (cold)                                                            [0x7f74744bd2c0] bci=[-1,18,12] rc=0 vc=1689 vn=- li=- udi=- nc=0
n326n     BBStart <block_15> (freq 2) (extension of previous block) (cold)                    [0x7f74744bc5a0] bci=[-1,22,12] rc=0 vc=1689 vn=- li=- udi=- nc=0
n961n     istore  <auto slot 2>[#421  Auto] [flags 0x3 0x0 ]                                  [0x7f7474678c20] bci=[-1,110,27] rc=0 vc=1708 vn=- li=- udi=14 nc=1
n962n       iadd                                                                              [0x7f7474678c70] bci=[-1,109,27] rc=1 vc=1708 vn=- li=- udi=- nc=2
n963n         iload  <auto slot 0>[#419  Auto] [flags 0x3 0x0 ] (cannotOverflow )             [0x7f7474678cc0] bci=[-1,104,27] rc=1 vc=1711 vn=- li=- udi=35 nc=0 flg=0x1000
n964n         iload  <temp slot 13>[#516  Auto] [flags 0x3 0x0 ] (cannotOverflow )            [0x7f7474678d10] bci=[-1,105,27] rc=1 vc=1710 vn=- li=- udi=36 nc=0 flg=0x1000
n325n     goto --> block_8 BBStart at n1n                                                     [0x7f74744bc550] bci=[-1,22,12] rc=0 vc=1702 vn=- li=- udi=- nc=0
n327n     BBEnd </block_15> (cold) =====                                                      [0x7f74744bc5f0] bci=[-1,22,12] rc=0 vc=1689 vn=- li=- udi=- nc=0

Notice that the commoned load of #419 that appeared in node n231n in the original tree has been replaced with a load of a temporary variable, #516, in the sunk tree beneath node n961n. #516 contains the value of #419 that had been loaded by n231n. However, the other commoned load of #419 has simply been copied into the sunk tree, so it is loading the value of #419 after the store in n235n, rather than the value before that store.

Rerunning the test after applying this change, yields

Trees before General Store Sinking and snippets from trace:
n406n     BBStart <block_27> (freq 2) (cold) (in loop 5)                                      [0x7f36aa5b8ea0] bci=[-1,18,12] rc=0 vc=1658 vn=- li=- udi=- nc=0
n236n     treetop                                                                             [0x7f36aa5b5980] bci=[-1,106,27] rc=0 vc=1658 vn=- li=- udi=- nc=1
n230n       iload  <auto slot 0>[#419  Auto] [flags 0x3 0x0 ] (cannotOverflow )               [0x7f36aa5b57a0] bci=[-1,104,27] rc=2 vc=1658 vn=- li=- udi=35 nc=0 flg=0x1000
n237n     treetop                                                                             [0x7f36aa5b59d0] bci=[-1,106,27] rc=0 vc=1658 vn=- li=- udi=- nc=1
n231n       iload  <auto slot 0>[#419  Auto] [flags 0x3 0x0 ] (cannotOverflow )               [0x7f36aa5b57f0] bci=[-1,105,27] rc=2 vc=1658 vn=- li=- udi=36 nc=0 flg=0x1000
n235n     istore  <auto slot 0>[#419  Auto] [flags 0x3 0x0 ]                                  [0x7f36aa5b5930] bci=[-1,106,27] rc=0 vc=1658 vn=- li=25 udi=13 nc=1
n234n       isub                                                                              [0x7f36aa5b58e0] bci=[-1,106,27] rc=1 vc=1658 vn=- li=- udi=- nc=2
n232n         iload  <auto slot 0>[#419  Auto] [flags 0x3 0x0 ] (cannotOverflow )             [0x7f36aa5b5840] bci=[-1,106,27] rc=1 vc=1658 vn=- li=- udi=37 nc=0 flg=0x1000
n233n         iconst -1 (X!=0 X<=0 )                                                          [0x7f36aa5b5890] bci=[-1,106,27] rc=1 vc=1658 vn=- li=- udi=- nc=0 flg=0x204
n239n     istore  <auto slot 2>[#421  Auto] [flags 0x3 0x0 ]                                  [0x7f36aa5b5a70] bci=[-1,110,27] rc=0 vc=1658 vn=- li=26 udi=14 nc=1
n238n       iadd                                                                              [0x7f36aa5b5a20] bci=[-1,109,27] rc=1 vc=1658 vn=- li=- udi=- nc=2
n230n         ==>iload
n231n         ==>iload
...
n365n     BBEnd </block_27> (cold)                                                            [0x7f36aa5b81d0] bci=[-1,18,12] rc=0 vc=1658 vn=- li=- udi=- nc=0
n326n     BBStart <block_15> (freq 2) (extension of previous block) (cold) (in loop 3)        [0x7f36aa5b75a0] bci=[-1,22,12] rc=0 vc=1658 vn=- li=- udi=- nc=0
n325n     goto --> block_8 BBStart at n1n                                                     [0x7f36aa5b7550] bci=[-1,22,12] rc=0 vc=1658 vn=- li=- udi=- nc=0
n327n     BBEnd </block_15> (cold) =====                                                      [0x7f36aa5b75f0] bci=[-1,22,12] rc=0 vc=1658 vn=- li=- udi=- nc=0

...

#419:  <auto slot 0>[ Auto] [flags 0x3 0x0 ] [0x7f36aa582ce0] (Int32)
...
Local #23 is symbol at 00007F36AA582CE0
...
    Examining node [00007F36AA5B5A70] in block_27
      savedLiveCommonedLoads: {}
      calling findLocalUses on node 00007F36AA5B5A70 with treeVisitCount 1688
         Looking backward for uses in node 00007F36AA5B5A70 has future use count = 0 and reference count = 0
            first reference, and this was the first of all commoned references to this sym
         Looking backward for uses in node 00007F36AA5B5A20 has future use count = 1 and reference count = 1
            first reference, and this was the first of all commoned references to this sym
         Looking backward for uses in node 00007F36AA5B57A0 [n230n] has future use count = 2 and reference count = 2
            not first reference
            Node [00007F36AA5B57A0] local [00007F36AA582CE0] idx 23
              Marking as commoned load
              Marking 23 as live commoned load
         Looking backward for uses in node 00007F36AA5B57F0 [n231n] has future use count = 2 and reference count = 2
            not first reference
            Node [00007F36AA5B57F0] local [00007F36AA582CE0] idx 23
              Marking as commoned load
              Marking 23 as live commoned load
      killedSymbols: {}
      usedSymbols: {}      treeCommonedLoads: {23}
      setting 21 in killedSymbols
      killedSymbols: {21}
      usedSymbols: {}
      treeCommonedLoads: {23}
      savedLiveCommonedLoads: {}
      killedLiveCommonedLoads: {}
      is store to local 21
         store passes some interfering and candidate tests
      creating use or kill info on movable node [00007F36AA5B5A70] to track kills and uses
      store is potentially movable, collecting commoned loads and adding to list
...
    Examining node [00007F36AA5B57F0] [n231n] in block_27
      savedLiveCommonedLoads: {23}
      calling findLocalUses on node 00007F36AA5B59D0 with treeVisitCount 1690
         Looking backward for uses in node 00007F36AA5B59D0 has future use count = 0 and reference count = 0
            first reference, and this was the first of all commoned references to this sym
         Looking backward for uses in node 00007F36AA5B57F0 has future use count = 1 and reference count = 2
            first reference, but other commoned references to this sym are still live
            Node [00007F36AA5B57F0] local [00007F36AA582CE0] idx 23
            Gening symbol with local index 23
      killedSymbols: {}
      usedSymbols: {23}      treeCommonedLoads: {}
      killedSymbols: {}
      usedSymbols: {23}
      treeCommonedLoads: {}
      savedLiveCommonedLoads: {23}
      killedLiveCommonedLoads: {23}
      symbol stores to a live commoned reference, searching stores below that load this symbol via commoned reference ...
            examining store [00007F36AA5B5BB0]
            examining store [00007F36AA5B5A70]
         (non-commoned) uses of killed symbols: {23}
            found store [00007F36AA5B5A70] below that may require a temp
               killed live commoned loads used above store:{23}
           found commonedLoad = 00007F36AA5B57F0
(Transformation #0 start - create temp store)
(Transformation #0 was performed)
      creating use or kill info on non-movable node [00007F36AA5B59D0] to track kills and uses
    skipping created temp store [00007F36AA773400] in backward treetop walk
    Examining node [00007F36AA5B57A0] [n230n] in block_27
      savedLiveCommonedLoads: {23}
      calling findLocalUses on node 00007F36AA5B5980 with treeVisitCount 1691
         Looking backward for uses in node 00007F36AA5B5980 has future use count = 0 and reference count = 0
            first reference, and this was the first of all commoned references to this sym
         Looking backward for uses in node 00007F36AA5B57A0 has future use count = 1 and reference count = 2
            first reference, and this was the first of all commoned references to this sym
            Node [00007F36AA5B57A0] local [00007F36AA582CE0] idx 23
            Gening symbol with local index 23
      killedSymbols: {}
      usedSymbols: {23}      treeCommonedLoads: {}
      killedSymbols: {}
      usedSymbols: {23}
      treeCommonedLoads: {}
      savedLiveCommonedLoads: {23}
      killedLiveCommonedLoads: {23}
      symbol stores to a live commoned reference, searching stores below that load this symbol via commoned reference ...
            examining store [00007F36AA5B5BB0]
            examining store [00007F36AA5B5A70]
         (non-commoned) uses of killed symbols: {23}
            found store [00007F36AA5B5A70] below that may require a temp
               killed live commoned loads used above store:{23}
           found commonedLoad = 00007F36AA5B57A0
(Transformation #1 start - create temp store)
(Transformation #1 was performed)
      creating use or kill info on non-movable node [00007F36AA5B5980] to track kills and uses
    skipping created temp store [00007F36AA773450] in backward treetop walk
  Reached beginning of extended basic block

Notice now that upon reaching n231n during the backwards tree walk, the optimization now recognizes that although that is the first instance of that node from the beginning of the method, it is not the first live commoned load of local symbol 23 (#419). That ensures a second temporary variable is created when the first instance of n230n from the beinning of the method is reached.

Trees after General Store Sinking
n406n     BBStart <block_27> (freq 2) (cold)                                                  [0x7f36aa5b8ea0] bci=[-1,18,12] rc=0 vc=1668 vn=- li=- udi=- nc=0
n936n     istore  <temp slot 14>[#515  Auto] [flags 0x3 0x0 ]                                 [0x7f36aa773450] bci=[-1,104,27] rc=0 vc=0 vn=- li=- udi=- nc=1
n230n       iload  <auto slot 0>[#419  Auto] [flags 0x3 0x0 ] (cannotOverflow )               [0x7f36aa5b57a0] bci=[-1,104,27] rc=3 vc=1691 vn=- li=- udi=35 nc=0 flg=0x1000
n236n     treetop                                                                             [0x7f36aa5b5980] bci=[-1,106,27] rc=0 vc=1691 vn=- li=- udi=- nc=1
n230n       ==>iload
n935n     istore  <temp slot 13>[#514  Auto] [flags 0x3 0x0 ]                                 [0x7f36aa773400] bci=[-1,105,27] rc=0 vc=0 vn=- li=- udi=- nc=1
n231n       iload  <auto slot 0>[#419  Auto] [flags 0x3 0x0 ] (cannotOverflow )               [0x7f36aa5b57f0] bci=[-1,105,27] rc=3 vc=1690 vn=- li=- udi=36 nc=0 flg=0x1000
n237n     treetop                                                                             [0x7f36aa5b59d0] bci=[-1,106,27] rc=0 vc=1690 vn=- li=- udi=- nc=1
n231n       ==>iload
n235n     istore  <auto slot 0>[#419  Auto] [flags 0x3 0x0 ]                                  [0x7f36aa5b5930] bci=[-1,106,27] rc=0 vc=1689 vn=- li=- udi=13 nc=1
n234n       isub                                                                              [0x7f36aa5b58e0] bci=[-1,106,27] rc=1 vc=1689 vn=- li=- udi=- nc=2
n232n         iload  <auto slot 0>[#419  Auto] [flags 0x3 0x0 ] (cannotOverflow )             [0x7f36aa5b5840] bci=[-1,106,27] rc=1 vc=1689 vn=- li=- udi=37 nc=0 flg=0x1000
n233n         iconst -1 (X!=0 X<=0 )                                                          [0x7f36aa5b5890] bci=[-1,106,27] rc=1 vc=1689 vn=- li=- udi=- nc=0 flg=0x204
n239n     treetop                                                                             [0x7f36aa5b5a70] bci=[-1,110,27] rc=0 vc=1688 vn=- li=- udi=14 nc=1
n238n       iadd                                                                              [0x7f36aa5b5a20] bci=[-1,109,27] rc=1 vc=1688 vn=- li=- udi=- nc=2
n230n         ==>iload
n231n         ==>iload
...
n365n     BBEnd </block_27> (cold)                                                            [0x7f36aa5b81d0] bci=[-1,18,12] rc=0 vc=1668 vn=- li=- udi=- nc=0
n326n     BBStart <block_15> (freq 2) (extension of previous block) (cold)                    [0x7f36aa5b75a0] bci=[-1,22,12] rc=0 vc=1668 vn=- li=- udi=- nc=0
n957n     istore  <auto slot 2>[#421  Auto] [flags 0x3 0x0 ]                                  [0x7f36aa773ae0] bci=[-1,110,27] rc=0 vc=1688 vn=- li=- udi=14 nc=1
n958n       iadd                                                                              [0x7f36aa773b30] bci=[-1,109,27] rc=1 vc=1688 vn=- li=- udi=- nc=2
n959n         iload  <temp slot 14>[#515  Auto] [flags 0x3 0x0 ] (cannotOverflow )            [0x7f36aa773b80] bci=[-1,104,27] rc=1 vc=1691 vn=- li=- udi=35 nc=0 flg=0x1000
n960n         iload  <temp slot 13>[#514  Auto] [flags 0x3 0x0 ] (cannotOverflow )            [0x7f36aa773bd0] bci=[-1,105,27] rc=1 vc=1690 vn=- li=- udi=36 nc=0 flg=0x1000
n325n     goto --> block_8 BBStart at n1n                                                     [0x7f36aa5b7550] bci=[-1,22,12] rc=0 vc=1682 vn=- li=- udi=- nc=0
n327n     BBEnd </block_15> (cold) =====                                                      [0x7f36aa5b75f0] bci=[-1,22,12] rc=0 vc=1668 vn=- li=- udi=- nc=0

@hzongaro
hzongaro marked this pull request as ready for review July 15, 2025 02:18
@hzongaro
hzongaro requested a review from vijaysun-omr as a code owner July 15, 2025 02:18
@hzongaro

Copy link
Copy Markdown
Contributor Author

@vijaysun-omr, may I ask you to review this change?

Comment thread compiler/optimizer/LiveVariableInformation.cpp Outdated
A set of trace messages in the TR_LiveVariableInformation method,
visitTreeForLocals, report during a backwards tree walk whether a node
contains the first commoned reference for a load of a sym.  The way the
messages are currently reported, they might indicate that a node that
does not have a symbol reference contains the first commoned reference
to a sym.  Corrected this to ensure the message only mentions syms for
node that are known to contain references to local symbols.

Signed-off-by:  Henry Zongaro <zongaro@ca.ibm.com>
@vijaysun-omr vijaysun-omr self-assigned this Jul 15, 2025
@vijaysun-omr

vijaysun-omr commented Jul 21, 2025

Copy link
Copy Markdown
Contributor

I wonder if another implementation could have just one data structure, i.e. a list of commoned load nodes hung off an array that can be indexed by the local index. That way you may have been able to check the length of the list at the places where you check the "size" currently. The advantage I am thinking of is a minor one here, but I don't feel the change is needed in the PR before I merge it. If you thought about that and feel it won't work for some reason, I am interested.

@vijaysun-omr

Copy link
Copy Markdown
Contributor

Jenkins build all

@vijaysun-omr

Copy link
Copy Markdown
Contributor

Jenkins build riscv

@hzongaro

Copy link
Copy Markdown
Contributor Author

Risc-v failure appears to be infrastructure-related. It's been failing for all pull request builds recently.

@hzongaro

Copy link
Copy Markdown
Contributor Author

I wonder if another implementation could have just one data structure, i.e. a list of commoned load nodes hung off an array that can be indexed by the local index. That way you may have been able to check the length of the list at the places where you check the "size" currently. The advantage I am thinking of is a minor one here, but I don't feel the change is needed in the PR before I merge it. If you thought about that and feel it won't work for some reason, I am interested.

That's a good point; I hadn't considered that possibility. I could take a look at that as a potential clean-up item.

@vijaysun-omr

Copy link
Copy Markdown
Contributor

The sole failure is an infra issue. Merging

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

JDK11 OpenJ9 Gives Non-deterministic Results

2 participants