Discussion:
RFR (XS): 8213525: new unit test GetLocalVariable/LocalVars is not stable
s***@oracle.com
2018-11-12 10:22:34 UTC
Permalink
<html>
<head>

<meta http-equiv="content-type" content="text/html; charset=utf-8">
</head>
<body text="#000000" bgcolor="#FFFFFF">
<font face="Monaco">Please, review a fix for:<br>
  <a class="moz-txt-link-freetext" href="https://bugs.openjdk.java.net/browse/JDK-8213525">https://bugs.openjdk.java.net/browse/JDK-8213525</a><br>
<br>
Webrev:<br>
 
<a class="moz-txt-link-freetext" href="http://cr.openjdk.java.net/~sspitsyn/webrevs/2018/8213525-unstable-test.1/">http://cr.openjdk.java.net/~sspitsyn/webrevs/2018/8213525-unstable-test.1/</a><br>
<br>
<br>
Summary:<br>
  A couple of the checks in new unit test developed for </font><font
face="Monaco"><span title="JDK-8080406: VM_GetOrSetLocal doesn't
check local slot type against requested type "><a
href="https://bugs.openjdk.java.net/browse/JDK-8080406"
data-issue-key="JDK-8080406" class="issue-link link-title
resolution">JDK-8080406</a> is not stable.<br>
  It is expected that the type of the local intLoc returned by
the </span></font><span class="new">StackValueCollection has<br>
  to be T_CONFLICT as it is out of scope at the point where the
testLocals() is called:<br>
</span>
<pre><span class="new"> int staticMeth(byte byteArg, Object objArg, double dblArg, int intArg) {</span>
<span class="new"> testLocals(Thread.currentThread());</span>
<span class="new"> {</span>
<span class="new"> int intLoc = 9999;</span>
<span class="new"> intArg = intLoc;</span>
<span class="new"> }</span>
<span class="new"> return intArg;</span>
<span class="new"> }

But sometimes the type T_INT is returned instead of </span><span class="new">T_CONFLICT.
The fix is to disable the checks that can fail because of it.


Thanks,
Serguei
</span><span class="new"></span></pre>
<font face="Monaco"><span title="JDK-8080406: VM_GetOrSetLocal
doesn't check local slot type against requested type "><span
class="link-summary"></span></span></font>
</body>
</html>
JC Beyler
2018-11-12 17:35:40 UTC
Permalink
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
Post by s***@oracle.com
https://bugs.openjdk.java.net/browse/JDK-8213525
http://cr.openjdk.java.net/~sspitsyn/webrevs/2018/8213525-unstable-test.1/
A couple of the checks in new unit test developed for JDK-8080406
<https://bugs.openjdk.java.net/browse/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
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
s***@oracle.com
2018-11-13 04:05:33 UTC
Permalink
<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
</head>
<body text="#000000" bgcolor="#FFFFFF">
<div class="moz-cite-prefix">Hi Jc,<br>
<br>
<br>
Thank you a lot for reviewing!<br>
<br>
On 11/12/18 09:35, JC Beyler wrote:<br>
</div>
<blockquote type="cite"
cite="mid:CAF9BGBytoNf6UKUo_rh-yuT6LCBWw1jh8Q=***@mail.gmail.com">
<div dir="ltr">Hi Serguei,
<div><br>
</div>
<div>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").</div>
</div>
</blockquote>
<br>
I also normally do not like commented out code.<br>
In this particular case, I considered commented out lines as part of
comment.<br>
They explain what is removed better than any words. :)<br>
<br>
Okay, I've removed these lines with the comment.<br>
<br>
<br>
<blockquote type="cite"
cite="mid:CAF9BGBytoNf6UKUo_rh-yuT6LCBWw1jh8Q=***@mail.gmail.com">
<div dir="ltr">
<div>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?</div>
</div>
</blockquote>
<br>
No.<br>
There is no information from the JIT compiler to return errors when
the LVT is absent.<br>
Moreover, different compilers, modes or tiers differently represent
local values that are out of scope. <br>
<br>
Thanks,<br>
Serguei<br>
<br>
<blockquote type="cite"
cite="mid:CAF9BGBytoNf6UKUo_rh-yuT6LCBWw1jh8Q=***@mail.gmail.com">
<div dir="ltr">
<div><br>
</div>
<div>Thanks,</div>
<div>Jc</div>
<div><br>
</div>
<div><br>
</div>
</div>
<br>
<div class="gmail_quote">
<div dir="ltr">On Mon, Nov 12, 2018 at 2:25 AM <a
href="mailto:***@oracle.com" target="_blank"
moz-do-not-send="true">***@oracle.com</a> &lt;<a
href="mailto:***@oracle.com" target="_blank"
moz-do-not-send="true">***@oracle.com</a>&gt;
wrote:<br>
</div>
<blockquote class="gmail_quote">
<div> Please, review a fix for:<br>
  <a
class="m_55448593796088709m_-3494849714881105216moz-txt-link-freetext"
href="https://bugs.openjdk.java.net/browse/JDK-8213525"
target="_blank" moz-do-not-send="true">https://bugs.openjdk.java.net/browse/JDK-8213525</a><br>
<br>
Webrev:<br>
 
<a
class="m_55448593796088709m_-3494849714881105216moz-txt-link-freetext"
href="http://cr.openjdk.java.net/%7Esspitsyn/webrevs/2018/8213525-unstable-test.1/"
target="_blank" moz-do-not-send="true">http://cr.openjdk.java.net/~sspitsyn/webrevs/2018/8213525-unstable-test.1/</a><br>
<br>
<br>
Summary:<br>
  A couple of the checks in new unit test developed for <span
title="JDK-8080406: VM_GetOrSetLocal doesn't check local
slot type against requested type "><a
href="https://bugs.openjdk.java.net/browse/JDK-8080406"
class="m_55448593796088709m_-3494849714881105216issue-link
m_55448593796088709m_-3494849714881105216link-title
m_55448593796088709m_-3494849714881105216resolution"
target="_blank" moz-do-not-send="true">JDK-8080406</a>
is not stable.<br>
  It is expected that the type of the local intLoc
returned by the </span><span
class="m_55448593796088709m_-3494849714881105216new">StackValueCollection
has<br>
  to be T_CONFLICT as it is out of scope at the point
where the testLocals() is called:<br>
</span>
<pre><span class="m_55448593796088709m_-3494849714881105216new"> int staticMeth(byte byteArg, Object objArg, double dblArg, int intArg) {</span>
<span class="m_55448593796088709m_-3494849714881105216new"> testLocals(Thread.currentThread());</span>
<span class="m_55448593796088709m_-3494849714881105216new"> {</span>
<span class="m_55448593796088709m_-3494849714881105216new"> int intLoc = 9999;</span>
<span class="m_55448593796088709m_-3494849714881105216new"> intArg = intLoc;</span>
<span class="m_55448593796088709m_-3494849714881105216new"> }</span>
<span class="m_55448593796088709m_-3494849714881105216new"> return intArg;</span>
<span class="m_55448593796088709m_-3494849714881105216new"> }

But sometimes the type T_INT is returned instead of </span><span class="m_55448593796088709m_-3494849714881105216new">T_CONFLICT.
The fix is to disable the checks that can fail because of it.


Thanks,
Serguei
</span><span class="m_55448593796088709m_-3494849714881105216new"></span></pre>
<span title="JDK-8080406: VM_GetOrSetLocal doesn't check
local slot type against requested type "><span
class="m_55448593796088709m_-3494849714881105216link-summary"></span></span>
</div>
</blockquote>
</div>
<br>
<div><br>
</div>
-- <br>
<div dir="ltr" class="m_55448593796088709gmail_signature"
data-smartmail="gmail_signature">
<div dir="ltr">
<div><br>
</div>
Thanks,
<div>Jc</div>
</div>
</div>
</blockquote>
<br>
</body>
</html>
s***@oracle.com
2018-11-13 04:06:41 UTC
Permalink
<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
</head>
<body text="#000000" bgcolor="#FFFFFF">
<div class="moz-cite-prefix">On 11/12/18 20:05,
<a class="moz-txt-link-abbreviated" href="mailto:***@oracle.com">***@oracle.com</a> wrote:<br>
</div>
<blockquote type="cite"
cite="mid:10037cde-0680-e66e-4e4a-***@oracle.com">
<div class="moz-cite-prefix">Hi Jc,<br>
<br>
<br>
Thank you a lot for reviewing!<br>
<br>
On 11/12/18 09:35, JC Beyler wrote:<br>
</div>
<blockquote type="cite"
cite="mid:CAF9BGBytoNf6UKUo_rh-yuT6LCBWw1jh8Q=***@mail.gmail.com">
<div dir="ltr">Hi Serguei,
<div><br>
</div>
<div>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").</div>
</div>
</blockquote>
<br>
I also normally do not like commented out code.<br>
In this particular case, I considered commented out lines as part
of comment.<br>
They explain what is removed better than any words. :)<br>
<br>
Okay, I've removed these lines with the comment.<br>
</blockquote>
<br>
Forgot to tell that I've updated the same webrev:<br>
  <a
class="m_55448593796088709m_-3494849714881105216moz-txt-link-freetext"
href="http://cr.openjdk.java.net/%7Esspitsyn/webrevs/2018/8213525-unstable-test.1/"
target="_blank">http://cr.openjdk.java.net/~sspitsyn/webrevs/2018/8213525-unstable-test.1/</a><br>
<br>
<br>
Thanks,<br>
Serguei<br>
<br>
<blockquote type="cite"
cite="mid:10037cde-0680-e66e-4e4a-***@oracle.com">
<blockquote type="cite"
cite="mid:CAF9BGBytoNf6UKUo_rh-yuT6LCBWw1jh8Q=***@mail.gmail.com">
<div dir="ltr">
<div>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?</div>
</div>
</blockquote>
<br>
No.<br>
There is no information from the JIT compiler to return errors
when the LVT is absent.<br>
Moreover, different compilers, modes or tiers differently
represent local values that are out of scope. <br>
<br>
Thanks,<br>
Serguei<br>
<br>
<blockquote type="cite"
cite="mid:CAF9BGBytoNf6UKUo_rh-yuT6LCBWw1jh8Q=***@mail.gmail.com">
<div dir="ltr">
<div><br>
</div>
<div>Thanks,</div>
<div>Jc</div>
<div><br>
</div>
<div><br>
</div>
</div>
<br>
<div class="gmail_quote">
<div dir="ltr">On Mon, Nov 12, 2018 at 2:25 AM <a
href="mailto:***@oracle.com" target="_blank"
moz-do-not-send="true">***@oracle.com</a> &lt;<a
href="mailto:***@oracle.com" target="_blank"
moz-do-not-send="true">***@oracle.com</a>&gt;
wrote:<br>
</div>
<blockquote class="gmail_quote">
<div> Please, review a fix for:<br>
  <a
class="m_55448593796088709m_-3494849714881105216moz-txt-link-freetext"
href="https://bugs.openjdk.java.net/browse/JDK-8213525"
target="_blank" moz-do-not-send="true">https://bugs.openjdk.java.net/browse/JDK-8213525</a><br>
<br>
Webrev:<br>
  <a
class="m_55448593796088709m_-3494849714881105216moz-txt-link-freetext"
href="http://cr.openjdk.java.net/%7Esspitsyn/webrevs/2018/8213525-unstable-test.1/"
target="_blank" moz-do-not-send="true">http://cr.openjdk.java.net/~sspitsyn/webrevs/2018/8213525-unstable-test.1/</a><br>
<br>
<br>
Summary:<br>
  A couple of the checks in new unit test developed for <span
title="JDK-8080406: VM_GetOrSetLocal doesn't check local
slot type against requested type "><a
href="https://bugs.openjdk.java.net/browse/JDK-8080406"
class="m_55448593796088709m_-3494849714881105216issue-link
m_55448593796088709m_-3494849714881105216link-title
m_55448593796088709m_-3494849714881105216resolution"
target="_blank" moz-do-not-send="true">JDK-8080406</a>
is not stable.<br>
  It is expected that the type of the local intLoc
returned by the </span><span
class="m_55448593796088709m_-3494849714881105216new">StackValueCollection
has<br>
  to be T_CONFLICT as it is out of scope at the point
where the testLocals() is called:<br>
</span>
<pre><span class="m_55448593796088709m_-3494849714881105216new"> int staticMeth(byte byteArg, Object objArg, double dblArg, int intArg) {</span>
<span class="m_55448593796088709m_-3494849714881105216new"> testLocals(Thread.currentThread());</span>
<span class="m_55448593796088709m_-3494849714881105216new"> {</span>
<span class="m_55448593796088709m_-3494849714881105216new"> int intLoc = 9999;</span>
<span class="m_55448593796088709m_-3494849714881105216new"> intArg = intLoc;</span>
<span class="m_55448593796088709m_-3494849714881105216new"> }</span>
<span class="m_55448593796088709m_-3494849714881105216new"> return intArg;</span>
<span class="m_55448593796088709m_-3494849714881105216new"> }

But sometimes the type T_INT is returned instead of </span><span class="m_55448593796088709m_-3494849714881105216new">T_CONFLICT.
The fix is to disable the checks that can fail because of it.


Thanks,
Serguei
</span><span class="m_55448593796088709m_-3494849714881105216new"></span></pre>
<span title="JDK-8080406: VM_GetOrSetLocal doesn't check
local slot type against requested type "><span
class="m_55448593796088709m_-3494849714881105216link-summary"></span></span>
</div>
</blockquote>
</div>
<br>
<div><br>
</div>
-- <br>
<div dir="ltr" class="m_55448593796088709gmail_signature"
data-smartmail="gmail_signature">
<div dir="ltr">
<div><br>
</div>
Thanks,
<div>Jc</div>
</div>
</div>
</blockquote>
<br>
</blockquote>
<br>
</body>
</html>
JC Beyler
2018-11-13 16:59:05 UTC
Permalink
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
Post by s***@oracle.com
Hi Jc,
Thank you a lot for reviewing!
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.
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
Post by s***@oracle.com
https://bugs.openjdk.java.net/browse/JDK-8213525
http://cr.openjdk.java.net/~sspitsyn/webrevs/2018/8213525-unstable-test.1/
A couple of the checks in new unit test developed for JDK-8080406
<https://bugs.openjdk.java.net/browse/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
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
s***@oracle.com
2018-11-13 17:19:24 UTC
Permalink
<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
</head>
<body text="#000000" bgcolor="#FFFFFF">
<div class="moz-cite-prefix">Thanks a lot, Jc!<br>
Serguei<br>
<br>
On 11/13/18 08:59, JC Beyler wrote:<br>
</div>
<blockquote type="cite"
cite="mid:CAF9BGBwu9nqOM8O0djoca5-AWD8C0DDUrQJ+***@mail.gmail.com">
<div dir="ltr">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.<br>
<div><br>
</div>
<div>Webrev looks good to me now :)</div>
<div>Jc</div>
</div>
<br>
<div class="gmail_quote">
<div dir="ltr">On Mon, Nov 12, 2018 at 8:06 PM <a
href="mailto:***@oracle.com"
moz-do-not-send="true">***@oracle.com</a> &lt;<a
href="mailto:***@oracle.com"
moz-do-not-send="true">***@oracle.com</a>&gt;
wrote:<br>
</div>
<blockquote class="gmail_quote">
<div>
<div class="m_7785226472344045076moz-cite-prefix">On
11/12/18 20:05, <a
class="m_7785226472344045076moz-txt-link-abbreviated"
href="mailto:***@oracle.com" target="_blank"
moz-do-not-send="true">***@oracle.com</a>
wrote:<br>
</div>
<blockquote type="cite">
<div class="m_7785226472344045076moz-cite-prefix">Hi Jc,<br>
<br>
<br>
Thank you a lot for reviewing!<br>
<br>
On 11/12/18 09:35, JC Beyler wrote:<br>
</div>
<blockquote type="cite">
<div dir="ltr">Hi Serguei,
<div><br>
</div>
<div>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").</div>
</div>
</blockquote>
<br>
I also normally do not like commented out code.<br>
In this particular case, I considered commented out lines
as part of comment.<br>
They explain what is removed better than any words. :)<br>
<br>
Okay, I've removed these lines with the comment.<br>
</blockquote>
<br>
Forgot to tell that I've updated the same webrev:<br>
  <a
class="m_7785226472344045076m_55448593796088709m_-3494849714881105216moz-txt-link-freetext"
href="http://cr.openjdk.java.net/%7Esspitsyn/webrevs/2018/8213525-unstable-test.1/"
target="_blank" moz-do-not-send="true">http://cr.openjdk.java.net/~sspitsyn/webrevs/2018/8213525-unstable-test.1/</a><br>
<br>
<br>
Thanks,<br>
Serguei<br>
<br>
<blockquote type="cite">
<blockquote type="cite">
<div dir="ltr">
<div>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?</div>
</div>
</blockquote>
<br>
No.<br>
There is no information from the JIT compiler to return
errors when the LVT is absent.<br>
Moreover, different compilers, modes or tiers differently
represent local values that are out of scope. <br>
<br>
Thanks,<br>
Serguei<br>
<br>
<blockquote type="cite">
<div dir="ltr">
<div><br>
</div>
<div>Thanks,</div>
<div>Jc</div>
<div><br>
</div>
<div><br>
</div>
</div>
<br>
<div class="gmail_quote">
<div dir="ltr">On Mon, Nov 12, 2018 at 2:25 AM <a
href="mailto:***@oracle.com"
target="_blank" moz-do-not-send="true">***@oracle.com</a>
&lt;<a href="mailto:***@oracle.com"
target="_blank" moz-do-not-send="true">***@oracle.com</a>&gt;
wrote:<br>
</div>
<blockquote class="gmail_quote">
<div> Please, review a fix for:<br>
  <a
class="m_7785226472344045076m_55448593796088709m_-3494849714881105216moz-txt-link-freetext"
href="https://bugs.openjdk.java.net/browse/JDK-8213525" target="_blank"
moz-do-not-send="true">https://bugs.openjdk.java.net/browse/JDK-8213525</a><br>
<br>
Webrev:<br>
  <a
class="m_7785226472344045076m_55448593796088709m_-3494849714881105216moz-txt-link-freetext"
href="http://cr.openjdk.java.net/%7Esspitsyn/webrevs/2018/8213525-unstable-test.1/"
target="_blank" moz-do-not-send="true">http://cr.openjdk.java.net/~sspitsyn/webrevs/2018/8213525-unstable-test.1/</a><br>
<br>
<br>
Summary:<br>
  A couple of the checks in new unit test
developed for <span title="JDK-8080406:
VM_GetOrSetLocal doesn't check local slot type
against requested type "><a
href="https://bugs.openjdk.java.net/browse/JDK-8080406"
class="m_7785226472344045076m_55448593796088709m_-3494849714881105216issue-link
m_7785226472344045076m_55448593796088709m_-3494849714881105216link-title
m_7785226472344045076m_55448593796088709m_-3494849714881105216resolution"
target="_blank" moz-do-not-send="true">JDK-8080406</a>
is not stable.<br>
  It is expected that the type of the local
intLoc returned by the </span><span
class="m_7785226472344045076m_55448593796088709m_-3494849714881105216new">StackValueCollection
has<br>
  to be T_CONFLICT as it is out of scope at the
point where the testLocals() is called:<br>
</span>
<pre><span class="m_7785226472344045076m_55448593796088709m_-3494849714881105216new"> int staticMeth(byte byteArg, Object objArg, double dblArg, int intArg) {</span>
<span class="m_7785226472344045076m_55448593796088709m_-3494849714881105216new"> testLocals(Thread.currentThread());</span>
<span class="m_7785226472344045076m_55448593796088709m_-3494849714881105216new"> {</span>
<span class="m_7785226472344045076m_55448593796088709m_-3494849714881105216new"> int intLoc = 9999;</span>
<span class="m_7785226472344045076m_55448593796088709m_-3494849714881105216new"> intArg = intLoc;</span>
<span class="m_7785226472344045076m_55448593796088709m_-3494849714881105216new"> }</span>
<span class="m_7785226472344045076m_55448593796088709m_-3494849714881105216new"> return intArg;</span>
<span class="m_7785226472344045076m_55448593796088709m_-3494849714881105216new"> }

But sometimes the type T_INT is returned instead of </span><span class="m_7785226472344045076m_55448593796088709m_-3494849714881105216new">T_CONFLICT.
The fix is to disable the checks that can fail because of it.


Thanks,
Serguei
</span><span class="m_7785226472344045076m_55448593796088709m_-3494849714881105216new"></span></pre>
<span title="JDK-8080406: VM_GetOrSetLocal doesn't
check local slot type against requested type "><span
class="m_7785226472344045076m_55448593796088709m_-3494849714881105216link-summary"></span></span>
</div>
</blockquote>
</div>
<br>
<div><br>
</div>
-- <br>
<div dir="ltr"
class="m_7785226472344045076m_55448593796088709gmail_signature"
data-smartmail="gmail_signature">
<div dir="ltr">
<div><br>
</div>
Thanks,
<div>Jc</div>
</div>
</div>
</blockquote>
<br>
</blockquote>
<br>
</div>
</blockquote>
</div>
<br>
<div><br>
</div>
-- <br>
<div dir="ltr" class="gmail_signature"
data-smartmail="gmail_signature">
<div dir="ltr">
<div><br>
</div>
Thanks,
<div>Jc</div>
</div>
</div>
</blockquote>
<br>
</body>
</html>
Chris Plummer
2018-11-13 18:55:33 UTC
Permalink
<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
</head>
<body text="#000000" bgcolor="#FFFFFF">
<div class="moz-cite-prefix">Hi Serguei,<br>
<br>
Changes look good.<br>
<br>
Chris<br>
<br>
On 11/12/18 8:06 PM, <a class="moz-txt-link-abbreviated" href="mailto:***@oracle.com">***@oracle.com</a> wrote:<br>
</div>
<blockquote type="cite"
cite="mid:91cf3149-a138-fe4b-9a81-***@oracle.com">
<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
<div class="moz-cite-prefix">On 11/12/18 20:05, <a
class="moz-txt-link-abbreviated"
href="mailto:***@oracle.com"
moz-do-not-send="true">***@oracle.com</a> wrote:<br>
</div>
<blockquote type="cite"
cite="mid:10037cde-0680-e66e-4e4a-***@oracle.com">
<div class="moz-cite-prefix">Hi Jc,<br>
<br>
<br>
Thank you a lot for reviewing!<br>
<br>
On 11/12/18 09:35, JC Beyler wrote:<br>
</div>
<blockquote type="cite"
cite="mid:CAF9BGBytoNf6UKUo_rh-yuT6LCBWw1jh8Q=***@mail.gmail.com">
<div dir="ltr">Hi Serguei,
<div><br>
</div>
<div>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").</div>
</div>
</blockquote>
<br>
I also normally do not like commented out code.<br>
In this particular case, I considered commented out lines as
part of comment.<br>
They explain what is removed better than any words. :)<br>
<br>
Okay, I've removed these lines with the comment.<br>
</blockquote>
<br>
Forgot to tell that I've updated the same webrev:<br>
  <a
class="m_55448593796088709m_-3494849714881105216moz-txt-link-freetext"
href="http://cr.openjdk.java.net/%7Esspitsyn/webrevs/2018/8213525-unstable-test.1/"
target="_blank" moz-do-not-send="true">http://cr.openjdk.java.net/~sspitsyn/webrevs/2018/8213525-unstable-test.1/</a><br>
<br>
<br>
Thanks,<br>
Serguei<br>
<br>
<blockquote type="cite"
cite="mid:10037cde-0680-e66e-4e4a-***@oracle.com">
<blockquote type="cite"
cite="mid:CAF9BGBytoNf6UKUo_rh-yuT6LCBWw1jh8Q=***@mail.gmail.com">
<div dir="ltr">
<div>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?</div>
</div>
</blockquote>
<br>
No.<br>
There is no information from the JIT compiler to return errors
when the LVT is absent.<br>
Moreover, different compilers, modes or tiers differently
represent local values that are out of scope. <br>
<br>
Thanks,<br>
Serguei<br>
<br>
<blockquote type="cite"
cite="mid:CAF9BGBytoNf6UKUo_rh-yuT6LCBWw1jh8Q=***@mail.gmail.com">
<div dir="ltr">
<div><br>
</div>
<div>Thanks,</div>
<div>Jc</div>
<div><br>
</div>
<div><br>
</div>
</div>
<br>
<div class="gmail_quote">
<div dir="ltr">On Mon, Nov 12, 2018 at 2:25 AM <a
href="mailto:***@oracle.com" target="_blank"
moz-do-not-send="true">***@oracle.com</a>
&lt;<a href="mailto:***@oracle.com"
target="_blank" moz-do-not-send="true">***@oracle.com</a>&gt;
wrote:<br>
</div>
<blockquote class="gmail_quote">
<div> Please, review a fix for:<br>
  <a
class="m_55448593796088709m_-3494849714881105216moz-txt-link-freetext"
href="https://bugs.openjdk.java.net/browse/JDK-8213525" target="_blank"
moz-do-not-send="true">https://bugs.openjdk.java.net/browse/JDK-8213525</a><br>
<br>
Webrev:<br>
  <a
class="m_55448593796088709m_-3494849714881105216moz-txt-link-freetext"
href="http://cr.openjdk.java.net/%7Esspitsyn/webrevs/2018/8213525-unstable-test.1/"
target="_blank" moz-do-not-send="true">http://cr.openjdk.java.net/~sspitsyn/webrevs/2018/8213525-unstable-test.1/</a><br>
<br>
<br>
Summary:<br>
  A couple of the checks in new unit test developed for
<span title="JDK-8080406: VM_GetOrSetLocal doesn't check
local slot type against requested type "><a
href="https://bugs.openjdk.java.net/browse/JDK-8080406"
class="m_55448593796088709m_-3494849714881105216issue-link
m_55448593796088709m_-3494849714881105216link-title
m_55448593796088709m_-3494849714881105216resolution"
target="_blank" moz-do-not-send="true">JDK-8080406</a>
is not stable.<br>
  It is expected that the type of the local intLoc
returned by the </span><span
class="m_55448593796088709m_-3494849714881105216new">StackValueCollection
has<br>
  to be T_CONFLICT as it is out of scope at the point
where the testLocals() is called:<br>
</span>
<pre><span class="m_55448593796088709m_-3494849714881105216new"> int staticMeth(byte byteArg, Object objArg, double dblArg, int intArg) {</span>
<span class="m_55448593796088709m_-3494849714881105216new"> testLocals(Thread.currentThread());</span>
<span class="m_55448593796088709m_-3494849714881105216new"> {</span>
<span class="m_55448593796088709m_-3494849714881105216new"> int intLoc = 9999;</span>
<span class="m_55448593796088709m_-3494849714881105216new"> intArg = intLoc;</span>
<span class="m_55448593796088709m_-3494849714881105216new"> }</span>
<span class="m_55448593796088709m_-3494849714881105216new"> return intArg;</span>
<span class="m_55448593796088709m_-3494849714881105216new"> }

But sometimes the type T_INT is returned instead of </span><span class="m_55448593796088709m_-3494849714881105216new">T_CONFLICT.
The fix is to disable the checks that can fail because of it.


Thanks,
Serguei
</span><span class="m_55448593796088709m_-3494849714881105216new"></span></pre>
<span title="JDK-8080406: VM_GetOrSetLocal doesn't check
local slot type against requested type "><span
class="m_55448593796088709m_-3494849714881105216link-summary"></span></span>
</div>
</blockquote>
</div>
<br>
<div><br>
</div>
-- <br>
<div dir="ltr" class="m_55448593796088709gmail_signature"
data-smartmail="gmail_signature">
<div dir="ltr">
<div><br>
</div>
Thanks,
<div>Jc</div>
</div>
</div>
</blockquote>
<br>
</blockquote>
<br>
</blockquote>
<p><br>
</p>
</body>
</html>
s***@oracle.com
2018-11-13 18:58:00 UTC
Permalink
Thanks a lot, Chris!
Serguei
Post by JC Beyler
Hi Serguei,
Changes look good.
Chris
Post by s***@oracle.com
Post by s***@oracle.com
Hi Jc,
Thank you a lot for reviewing!
Post by 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").
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.
http://cr.openjdk.java.net/~sspitsyn/webrevs/2018/8213525-unstable-test.1/
<http://cr.openjdk.java.net/%7Esspitsyn/webrevs/2018/8213525-unstable-test.1/>
Thanks,
Serguei
Post by s***@oracle.com
Post by JC Beyler
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
Post by JC Beyler
Thanks,
Jc
https://bugs.openjdk.java.net/browse/JDK-8213525
http://cr.openjdk.java.net/~sspitsyn/webrevs/2018/8213525-unstable-test.1/
<http://cr.openjdk.java.net/%7Esspitsyn/webrevs/2018/8213525-unstable-test.1/>
  A couple of the checks in new unit test developed for
JDK-8080406 <https://bugs.openjdk.java.net/browse/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
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
Loading...