openj9 icon indicating copy to clipboard operation
openj9 copied to clipboard

Privatized argument rematerialization can incorrectly ignore commoning

Open jdmpapin opened this issue 2 years ago • 0 comments

Consider the go() method in the following Java program:

public class RematDup {
    private static RematDup inst;

    int field;

    public static void main(String[] args) {
        inst = new RematDup();
        inst.field = 42;
        System.out.println(go());
        System.out.println(go());
    }

    public static int go() {
        RematDup localInst = inst; // non-parameter to inhibit prex
        // edit bytecode to use dup/swap instead of local variable a
        int a = localInst.field;
        return a - localInst.inlineFoo(a);
    }

    public int inlineFoo(int x) {
        return x;
    }
}

I've edited the class file as mentioned in the comment. This method loads field just once, and uses the resulting value as both the first operand of the subtraction and as the argument to inlineFoo(). After compiling:

limit={*.go(*)*},count=1,disableAsyncCompilation,initialOptLevel=warm,inhibitRecompilation,tryToInline={*.inline*}

the commoning is visible in the trees, where field is loaded as n6n, and the result is used in both places:

n4n       astore  localInst<auto slot 0>[#382  Auto]
n3n         aload  RematDup.inst LRematDup;[#381  Static]
n7n       NULLCHK on n5n [#32]
n6n         iloadi  RematDup.field I[#383  Shadow +8]
n5n           aload  localInst<auto slot 0>[#382  Auto]
n11n      NULLCHK on n8n [#32]
n10n        icalli  RematDup.inlineFoo(I)I[#386  virtual Method -64]
n9n           aloadi  <vft-symbol>[#309  Shadow]
n8n             aload  localInst<auto slot 0>[#382  Auto]
n8n           ==>aload
n6n           ==>iloadi
n13n      ireturn
n12n        isub
n6n           ==>iloadi
n10n          ==>icalli

Privatized argument rematerialization introduces a second load on the cold path. If the instance were writable from another thread (which it could be, as far as the JIT knows), then in the presence of a data race it would be possible for the second load to see a different value from the first, in which case the argument value would (incorrectly) differ from the first subtraction operand.

jdmpapin avatar Sep 15 '22 22:09 jdmpapin