[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-hotspot-compiler-dev
Subject:    Re: RFR(M) : 8134102 : [TESTBUG] compiler/unsafe/UnsafeGetConstantField.java test fails in Jake
From:       Vladimir Ivanov <vladimir.x.ivanov () oracle ! com>
Date:       2016-01-29 12:24:09
Message-ID: 56AB59E9.9000606 () oracle ! com
[Download RAW message or body]

Overall, looks good.

One request: JDK-8148518 is caused by field and getter type mismatch [1] 
(char vs short). Such behavior is expected, since char & short loads 
aren't interchangeable [2].

There are different ways to fix that particular case (add new intrinsic 
or enhance constant folding logic to take the cast into account), but 
for now, please, change the filter to ignore CharUnaligned and add a 
comment with bug id (JDK-8148518):
+            if (!hasDefaultValue && (stable || g.isFinal())) {

Best regards,
Vladimir Ivanov

[1] jdk/src/java.base/share/classes/jdk/internal/misc/Unsafe.java:
     @HotSpotIntrinsicCandidate
     public final char getCharUnaligned(Object o, long offset) {
         return (char)getShortUnaligned(o, offset);
     }

[2] 
http://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/2015-June/018322.html
"I spotted a bug when field and accessor types mismatch, but the JIT 
still constant-folds the load. The fix made expected result detection 
even more complex, so I decided to get rid of it & WhiteBox hooks 
altogether. The test exercises different code paths and compares 
returned values now."

On 1/29/16 4:49 AM, Igor Ignatyev wrote:
> http://cr.openjdk.java.net/~iignatyev/8134102/webrev.00/
> > 134 lines changed: 84 ins; 15 del; 35 mod;
> Hi all,
> 
> could you please review the patch for compiler/unsafe/UnsafeGetConstantField.java \
> test? 
> the test fails in jake, because the test class is in java.lang.invoke package, \
> which is already defined in java.base module. Instead of using "patch" mechanism, \
> which allows to add classes into existing modules, I decided to remove direct usage \
>                 of package-private members from j.l.i:
> - @DontInline was changed by a corresponding -XX:CompileCommand
> - direct usage of Stable.class replaced w/ Class.forName
> - UnsafeGetConstantField is moved from java.lang.invoke package to compile.unsafe, \
> thus all the nested classes used from generated tests are made public 
> Besides changes for jake, I also slightly modified the test (originally to be sure \
>                 that the test still checks that it supposed to):
> - for getObject* tests, String constant is used as the field value instead of 'new \
>                 Object()'
> - add checks that Test::testDirect/testUnsafe return prev. value even after field's \
> value was changed. this check fails for Unsafe::getCharUnaligned if JVM is started \
> w/ -XX:-UseUnalignedAccesses. I've filed a bug for that (JDK-8148518) and temporary \
>                 disabled the check which fails
> - in case of failure, the generated class is dumped into workdir
> 
> testing: run the test against 2016-01-26 jake nightly build
> 
> JDK-8134102 : http://bugs.openjdk.java.net/browse/JDK-8134102
> JDK-8148518 : http://bugs.openjdk.java.net/browse/JDK-8148518
> 
> PS the patch will be integrated thru jigsaw/jake repo
> 
> Thanks,
> — Igor
> 


[prev in list] [next in list] [prev in thread] [next in thread]