RFR (XS): 8213525: new unit test GetLocalVariable/LocalVars is not stable

classic Classic list List threaded Threaded
8 messages Options
Reply | Threaded
Open this post in threaded view
|

RFR (XS): 8213525: new unit test GetLocalVariable/LocalVars is not stable

serguei.spitsyn@oracle.com
Please, review a fix for:
  https://bugs.openjdk.java.net/browse/JDK-8213525

Webrev:
  http://cr.openjdk.java.net/~sspitsyn/webrevs/2018/8213525-unstable-test.1/


Summary:
  A couple of the checks in new unit test developed for
JDK-8080406 is not stable.
  It is expected that the type of the local intLoc returned by the
StackValueCollection has
  to be T_CONFLICT as it is out of scope at the point where the testLocals() is called:
     int staticMeth(byte byteArg, Object objArg, double dblArg, int intArg) {
         testLocals(Thread.currentThread());
         {
             int intLoc = 9999;
             intArg = intLoc;
         }
         return intArg;
     }

  But sometimes the type T_INT is returned instead of T_CONFLICT.
  The fix is to disable the checks that can fail because of it.


Thanks,
Serguei
Reply | Threaded
Open this post in threaded view
|

Re: RFR (XS): 8213525: new unit test GetLocalVariable/LocalVars is not stable

JC Beyler
Hi Serguei,

The fix looks good (though I never like commented out code, why do we not just remove the lines and add a simple comment: "Due to JDK-8213525, we do not test X,Y, and Z because of stability isssues").

But the underlying question I have that is not really explained is : "why is it failing?"; is the spec not specific in these cases? is it a bug in the compiler/runtime that is not yet fixed to conform to the spec? I ask because I would imagine that it might be something we would like to fix, no?

Thanks,
Jc



On Mon, Nov 12, 2018 at 2:25 AM [hidden email] <[hidden email]> wrote:
Please, review a fix for:
  https://bugs.openjdk.java.net/browse/JDK-8213525

Webrev:
  http://cr.openjdk.java.net/~sspitsyn/webrevs/2018/8213525-unstable-test.1/


Summary:
  A couple of the checks in new unit test developed for
JDK-8080406 is not stable.
  It is expected that the type of the local intLoc returned by the
StackValueCollection has
  to be T_CONFLICT as it is out of scope at the point where the testLocals() is called:
     int staticMeth(byte byteArg, Object objArg, double dblArg, int intArg) {
         testLocals(Thread.currentThread());
         {
             int intLoc = 9999;
             intArg = intLoc;
         }
         return intArg;
     }

  But sometimes the type T_INT is returned instead of T_CONFLICT.
  The fix is to disable the checks that can fail because of it.


Thanks,
Serguei


--

Thanks,
Jc
Reply | Threaded
Open this post in threaded view
|

Re: RFR (XS): 8213525: new unit test GetLocalVariable/LocalVars is not stable

serguei.spitsyn@oracle.com
Hi Jc,


Thank you a lot for reviewing!

On 11/12/18 09:35, JC Beyler wrote:
Hi Serguei,

The fix looks good (though I never like commented out code, why do we not just remove the lines and add a simple comment: "Due to JDK-8213525, we do not test X,Y, and Z because of stability isssues").

I also normally do not like commented out code.
In this particular case, I considered commented out lines as part of comment.
They explain what is removed better than any words. :)

Okay, I've removed these lines with the comment.


But the underlying question I have that is not really explained is : "why is it failing?"; is the spec not specific in these cases? is it a bug in the compiler/runtime that is not yet fixed to conform to the spec? I ask because I would imagine that it might be something we would like to fix, no?

No.
There is no information from the JIT compiler to return errors when the LVT is absent.
Moreover, different compilers, modes or tiers differently represent local values that are out of scope.

Thanks,
Serguei


Thanks,
Jc



On Mon, Nov 12, 2018 at 2:25 AM [hidden email] <[hidden email]> wrote:
Please, review a fix for:
  https://bugs.openjdk.java.net/browse/JDK-8213525

Webrev:
  http://cr.openjdk.java.net/~sspitsyn/webrevs/2018/8213525-unstable-test.1/


Summary:
  A couple of the checks in new unit test developed for JDK-8080406 is not stable.
  It is expected that the type of the local intLoc returned by the
StackValueCollection has
  to be T_CONFLICT as it is out of scope at the point where the testLocals() is called:
     int staticMeth(byte byteArg, Object objArg, double dblArg, int intArg) {
         testLocals(Thread.currentThread());
         {
             int intLoc = 9999;
             intArg = intLoc;
         }
         return intArg;
     }

  But sometimes the type T_INT is returned instead of T_CONFLICT.
  The fix is to disable the checks that can fail because of it.


Thanks,
Serguei


--

Thanks,
Jc

Reply | Threaded
Open this post in threaded view
|

Re: RFR (XS): 8213525: new unit test GetLocalVariable/LocalVars is not stable

serguei.spitsyn@oracle.com
On 11/12/18 20:05, [hidden email] wrote:
Hi Jc,


Thank you a lot for reviewing!

On 11/12/18 09:35, JC Beyler wrote:
Hi Serguei,

The fix looks good (though I never like commented out code, why do we not just remove the lines and add a simple comment: "Due to JDK-8213525, we do not test X,Y, and Z because of stability isssues").

I also normally do not like commented out code.
In this particular case, I considered commented out lines as part of comment.
They explain what is removed better than any words. :)

Okay, I've removed these lines with the comment.

Forgot to tell that I've updated the same webrev:
  http://cr.openjdk.java.net/~sspitsyn/webrevs/2018/8213525-unstable-test.1/


Thanks,
Serguei

But the underlying question I have that is not really explained is : "why is it failing?"; is the spec not specific in these cases? is it a bug in the compiler/runtime that is not yet fixed to conform to the spec? I ask because I would imagine that it might be something we would like to fix, no?

No.
There is no information from the JIT compiler to return errors when the LVT is absent.
Moreover, different compilers, modes or tiers differently represent local values that are out of scope.

Thanks,
Serguei


Thanks,
Jc



On Mon, Nov 12, 2018 at 2:25 AM [hidden email] <[hidden email]> wrote:
Please, review a fix for:
  https://bugs.openjdk.java.net/browse/JDK-8213525

Webrev:
  http://cr.openjdk.java.net/~sspitsyn/webrevs/2018/8213525-unstable-test.1/


Summary:
  A couple of the checks in new unit test developed for JDK-8080406 is not stable.
  It is expected that the type of the local intLoc returned by the
StackValueCollection has
  to be T_CONFLICT as it is out of scope at the point where the testLocals() is called:
     int staticMeth(byte byteArg, Object objArg, double dblArg, int intArg) {
         testLocals(Thread.currentThread());
         {
             int intLoc = 9999;
             intArg = intLoc;
         }
         return intArg;
     }

  But sometimes the type T_INT is returned instead of T_CONFLICT.
  The fix is to disable the checks that can fail because of it.


Thanks,
Serguei


--

Thanks,
Jc


Reply | Threaded
Open this post in threaded view
|

Re: RFR (XS): 8213525: new unit test GetLocalVariable/LocalVars is not stable

JC Beyler
Ok makes sense then: it is not specified what happens to locals that are out of scope and therefore depending on the compiler/modes/tiers, you could get a different return in those cases.

Webrev looks good to me now :)
Jc

On Mon, Nov 12, 2018 at 8:06 PM [hidden email] <[hidden email]> wrote:
On 11/12/18 20:05, [hidden email] wrote:
Hi Jc,


Thank you a lot for reviewing!

On 11/12/18 09:35, JC Beyler wrote:
Hi Serguei,

The fix looks good (though I never like commented out code, why do we not just remove the lines and add a simple comment: "Due to JDK-8213525, we do not test X,Y, and Z because of stability isssues").

I also normally do not like commented out code.
In this particular case, I considered commented out lines as part of comment.
They explain what is removed better than any words. :)

Okay, I've removed these lines with the comment.

Forgot to tell that I've updated the same webrev:
  http://cr.openjdk.java.net/~sspitsyn/webrevs/2018/8213525-unstable-test.1/


Thanks,
Serguei

But the underlying question I have that is not really explained is : "why is it failing?"; is the spec not specific in these cases? is it a bug in the compiler/runtime that is not yet fixed to conform to the spec? I ask because I would imagine that it might be something we would like to fix, no?

No.
There is no information from the JIT compiler to return errors when the LVT is absent.
Moreover, different compilers, modes or tiers differently represent local values that are out of scope.

Thanks,
Serguei


Thanks,
Jc



On Mon, Nov 12, 2018 at 2:25 AM [hidden email] <[hidden email]> wrote:
Please, review a fix for:
  https://bugs.openjdk.java.net/browse/JDK-8213525

Webrev:
  http://cr.openjdk.java.net/~sspitsyn/webrevs/2018/8213525-unstable-test.1/


Summary:
  A couple of the checks in new unit test developed for JDK-8080406 is not stable.
  It is expected that the type of the local intLoc returned by the
StackValueCollection has
  to be T_CONFLICT as it is out of scope at the point where the testLocals() is called:
     int staticMeth(byte byteArg, Object objArg, double dblArg, int intArg) {
         testLocals(Thread.currentThread());
         {
             int intLoc = 9999;
             intArg = intLoc;
         }
         return intArg;
     }

  But sometimes the type T_INT is returned instead of T_CONFLICT.
  The fix is to disable the checks that can fail because of it.


Thanks,
Serguei


--

Thanks,
Jc




--

Thanks,
Jc
Reply | Threaded
Open this post in threaded view
|

Re: RFR (XS): 8213525: new unit test GetLocalVariable/LocalVars is not stable

serguei.spitsyn@oracle.com
Thanks a lot, Jc!
Serguei

On 11/13/18 08:59, JC Beyler wrote:
Ok makes sense then: it is not specified what happens to locals that are out of scope and therefore depending on the compiler/modes/tiers, you could get a different return in those cases.

Webrev looks good to me now :)
Jc

On Mon, Nov 12, 2018 at 8:06 PM [hidden email] <[hidden email]> wrote:
On 11/12/18 20:05, [hidden email] wrote:
Hi Jc,


Thank you a lot for reviewing!

On 11/12/18 09:35, JC Beyler wrote:
Hi Serguei,

The fix looks good (though I never like commented out code, why do we not just remove the lines and add a simple comment: "Due to JDK-8213525, we do not test X,Y, and Z because of stability isssues").

I also normally do not like commented out code.
In this particular case, I considered commented out lines as part of comment.
They explain what is removed better than any words. :)

Okay, I've removed these lines with the comment.

Forgot to tell that I've updated the same webrev:
  http://cr.openjdk.java.net/~sspitsyn/webrevs/2018/8213525-unstable-test.1/


Thanks,
Serguei

But the underlying question I have that is not really explained is : "why is it failing?"; is the spec not specific in these cases? is it a bug in the compiler/runtime that is not yet fixed to conform to the spec? I ask because I would imagine that it might be something we would like to fix, no?

No.
There is no information from the JIT compiler to return errors when the LVT is absent.
Moreover, different compilers, modes or tiers differently represent local values that are out of scope.

Thanks,
Serguei


Thanks,
Jc



On Mon, Nov 12, 2018 at 2:25 AM [hidden email] <[hidden email]> wrote:
Please, review a fix for:
  https://bugs.openjdk.java.net/browse/JDK-8213525

Webrev:
  http://cr.openjdk.java.net/~sspitsyn/webrevs/2018/8213525-unstable-test.1/


Summary:
  A couple of the checks in new unit test developed for JDK-8080406 is not stable.
  It is expected that the type of the local intLoc returned by the
StackValueCollection has
  to be T_CONFLICT as it is out of scope at the point where the testLocals() is called:
     int staticMeth(byte byteArg, Object objArg, double dblArg, int intArg) {
         testLocals(Thread.currentThread());
         {
             int intLoc = 9999;
             intArg = intLoc;
         }
         return intArg;
     }

  But sometimes the type T_INT is returned instead of T_CONFLICT.
  The fix is to disable the checks that can fail because of it.


Thanks,
Serguei


--

Thanks,
Jc




--

Thanks,
Jc

Reply | Threaded
Open this post in threaded view
|

Re: RFR (XS): 8213525: new unit test GetLocalVariable/LocalVars is not stable

Chris Plummer
In reply to this post by serguei.spitsyn@oracle.com
Hi Serguei,

Changes look good.

Chris

On 11/12/18 8:06 PM, [hidden email] wrote:
On 11/12/18 20:05, [hidden email] wrote:
Hi Jc,


Thank you a lot for reviewing!

On 11/12/18 09:35, JC Beyler wrote:
Hi Serguei,

The fix looks good (though I never like commented out code, why do we not just remove the lines and add a simple comment: "Due to JDK-8213525, we do not test X,Y, and Z because of stability isssues").

I also normally do not like commented out code.
In this particular case, I considered commented out lines as part of comment.
They explain what is removed better than any words. :)

Okay, I've removed these lines with the comment.

Forgot to tell that I've updated the same webrev:
  http://cr.openjdk.java.net/~sspitsyn/webrevs/2018/8213525-unstable-test.1/


Thanks,
Serguei

But the underlying question I have that is not really explained is : "why is it failing?"; is the spec not specific in these cases? is it a bug in the compiler/runtime that is not yet fixed to conform to the spec? I ask because I would imagine that it might be something we would like to fix, no?

No.
There is no information from the JIT compiler to return errors when the LVT is absent.
Moreover, different compilers, modes or tiers differently represent local values that are out of scope.

Thanks,
Serguei


Thanks,
Jc



On Mon, Nov 12, 2018 at 2:25 AM [hidden email] <[hidden email]> wrote:
Please, review a fix for:
  https://bugs.openjdk.java.net/browse/JDK-8213525

Webrev:
  http://cr.openjdk.java.net/~sspitsyn/webrevs/2018/8213525-unstable-test.1/


Summary:
  A couple of the checks in new unit test developed for JDK-8080406 is not stable.
  It is expected that the type of the local intLoc returned by the
StackValueCollection has
  to be T_CONFLICT as it is out of scope at the point where the testLocals() is called:
     int staticMeth(byte byteArg, Object objArg, double dblArg, int intArg) {
         testLocals(Thread.currentThread());
         {
             int intLoc = 9999;
             intArg = intLoc;
         }
         return intArg;
     }

  But sometimes the type T_INT is returned instead of T_CONFLICT.
  The fix is to disable the checks that can fail because of it.


Thanks,
Serguei


--

Thanks,
Jc



Reply | Threaded
Open this post in threaded view
|

Re: RFR (XS): 8213525: new unit test GetLocalVariable/LocalVars is not stable

serguei.spitsyn@oracle.com
Thanks a lot, Chris!
Serguei

On 11/13/18 10:55 AM, Chris Plummer wrote:
Hi Serguei,

Changes look good.

Chris

On 11/12/18 8:06 PM, [hidden email] wrote:
On 11/12/18 20:05, [hidden email] wrote:
Hi Jc,


Thank you a lot for reviewing!

On 11/12/18 09:35, JC Beyler wrote:
Hi Serguei,

The fix looks good (though I never like commented out code, why do we not just remove the lines and add a simple comment: "Due to JDK-8213525, we do not test X,Y, and Z because of stability isssues").

I also normally do not like commented out code.
In this particular case, I considered commented out lines as part of comment.
They explain what is removed better than any words. :)

Okay, I've removed these lines with the comment.

Forgot to tell that I've updated the same webrev:
  http://cr.openjdk.java.net/~sspitsyn/webrevs/2018/8213525-unstable-test.1/


Thanks,
Serguei

But the underlying question I have that is not really explained is : "why is it failing?"; is the spec not specific in these cases? is it a bug in the compiler/runtime that is not yet fixed to conform to the spec? I ask because I would imagine that it might be something we would like to fix, no?

No.
There is no information from the JIT compiler to return errors when the LVT is absent.
Moreover, different compilers, modes or tiers differently represent local values that are out of scope.

Thanks,
Serguei


Thanks,
Jc



On Mon, Nov 12, 2018 at 2:25 AM [hidden email] <[hidden email]> wrote:
Please, review a fix for:
  https://bugs.openjdk.java.net/browse/JDK-8213525

Webrev:
  http://cr.openjdk.java.net/~sspitsyn/webrevs/2018/8213525-unstable-test.1/


Summary:
  A couple of the checks in new unit test developed for JDK-8080406 is not stable.
  It is expected that the type of the local intLoc returned by the
StackValueCollection has
  to be T_CONFLICT as it is out of scope at the point where the testLocals() is called:
     int staticMeth(byte byteArg, Object objArg, double dblArg, int intArg) {
         testLocals(Thread.currentThread());
         {
             int intLoc = 9999;
             intArg = intLoc;
         }
         return intArg;
     }

  But sometimes the type T_INT is returned instead of T_CONFLICT.
  The fix is to disable the checks that can fail because of it.


Thanks,
Serguei


--

Thanks,
Jc