<html xmlns:v="urn:schemas-microsoft-com:vml" xmlns:o="urn:schemas-microsoft-com:office:office" xmlns:w="urn:schemas-microsoft-com:office:word" xmlns:m="http://schemas.microsoft.com/office/2004/12/omml" xmlns="http://www.w3.org/TR/REC-html40">
<head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
<meta name="Generator" content="Microsoft Word 14 (filtered medium)">
<style><!--
/* Font Definitions */
@font-face
        {font-family:Calibri;
        panose-1:2 15 5 2 2 2 4 3 2 4;}
@font-face
        {font-family:Tahoma;
        panose-1:2 11 6 4 3 5 4 4 2 4;}
/* Style Definitions */
p.MsoNormal, li.MsoNormal, div.MsoNormal
        {margin:0cm;
        margin-bottom:.0001pt;
        font-size:12.0pt;
        font-family:"Times New Roman","serif";}
a:link, span.MsoHyperlink
        {mso-style-priority:99;
        color:blue;
        text-decoration:underline;}
a:visited, span.MsoHyperlinkFollowed
        {mso-style-priority:99;
        color:purple;
        text-decoration:underline;}
p.MsoAcetate, li.MsoAcetate, div.MsoAcetate
        {mso-style-priority:99;
        mso-style-link:"Balloon Text Char";
        margin:0cm;
        margin-bottom:.0001pt;
        font-size:8.0pt;
        font-family:"Tahoma","sans-serif";}
span.apple-converted-space
        {mso-style-name:apple-converted-space;}
span.EmailStyle18
        {mso-style-type:personal-reply;
        font-family:"Calibri","sans-serif";
        color:#1F497D;}
span.BalloonTextChar
        {mso-style-name:"Balloon Text Char";
        mso-style-priority:99;
        mso-style-link:"Balloon Text";
        font-family:"Tahoma","sans-serif";}
.MsoChpDefault
        {mso-style-type:export-only;
        font-size:10.0pt;}
@page WordSection1
        {size:612.0pt 792.0pt;
        margin:70.85pt 70.85pt 2.0cm 70.85pt;}
div.WordSection1
        {page:WordSection1;}
--></style><!--[if gte mso 9]><xml>
<o:shapedefaults v:ext="edit" spidmax="1026" />
</xml><![endif]--><!--[if gte mso 9]><xml>
<o:shapelayout v:ext="edit">
<o:idmap v:ext="edit" data="1" />
</o:shapelayout></xml><![endif]-->
</head>
<body lang="DE" link="blue" vlink="purple">
<div class="WordSection1">
<p class="MsoNormal"><span lang="EN-US" style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D">Hi Christian,<o:p></o:p></span></p>
<p class="MsoNormal"><span lang="EN-US" style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D"><o:p> </o:p></span></p>
<p class="MsoNormal"><span lang="EN-US" style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D">I have just added comments. We can also get rid of the multi-inheritance in RanceCheckEliminator::Verification.<o:p></o:p></span></p>
<p class="MsoNormal"><span lang="EN-US" style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D"><o:p> </o:p></span></p>
<p class="MsoNormal"><span lang="EN-US" style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D">The new webrev is:<o:p></o:p></span></p>
<p class="MsoNormal"><span lang="EN-US" style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D">http://cr.openjdk.java.net/~mdoerr/8138890_c1_ambiguous_delete/webrev.03</span><span lang="EN-US"><o:p></o:p></span></p>
<p class="MsoNormal"><span lang="EN-US"><o:p> </o:p></span></p>
<p class="MsoNormal"><span lang="EN-US" style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D">Best regards,</span><o:p></o:p></p>
<p class="MsoNormal"><span lang="EN-US" style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D">  Martin</span><o:p></o:p></p>
<p class="MsoNormal"><span lang="EN-US"><o:p> </o:p></span></p>
<div>
<div style="border:none;border-top:solid #B5C4DF 1.0pt;padding:3.0pt 0cm 0cm 0cm">
<p class="MsoNormal"><b><span lang="EN-US" style="font-size:10.0pt;font-family:"Tahoma","sans-serif"">From:</span></b><span lang="EN-US" style="font-size:10.0pt;font-family:"Tahoma","sans-serif""> Christian Thalinger [mailto:christian.thalinger@oracle.com]
<br>
<b>Sent:</b> Freitag, 9. Oktober 2015 20:35<br>
<b>To:</b> Doerr, Martin<br>
<b>Cc:</b> Volker Simonis; Lindenmaier, Goetz; Mikael Gerdin; hotspot compiler<br>
<b>Subject:</b> Re: RFR(S): 8138890: C1: Ambiguous operator delete<o:p></o:p></span></p>
</div>
</div>
<p class="MsoNormal"><o:p> </o:p></p>
<p class="MsoNormal"><o:p> </o:p></p>
<div>
<blockquote style="margin-top:5.0pt;margin-bottom:5.0pt">
<div>
<p class="MsoNormal">On Oct 9, 2015, at 12:03 AM, Doerr, Martin <<a href="mailto:martin.doerr@sap.com">martin.doerr@sap.com</a>> wrote:<o:p></o:p></p>
</div>
<p class="MsoNormal"><o:p> </o:p></p>
<div>
<div>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D">Hi,</span><o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D"> </span><o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><span lang="EN-US" style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D">thanks for reviewing.</span><o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><span lang="EN-US" style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D">As requested by Götz, I just removed the second ‘private’.</span><o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><span lang="EN-US" style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D">The new webrev is here:</span><o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><span lang="EN-US" style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D"><a href="http://cr.openjdk.java.net/~mdoerr/8138890_c1_ambiguous_delete/webrev.02"><span style="color:purple">http://cr.openjdk.java.net/~mdoerr/8138890_c1_ambiguous_delete/webrev.02</span></a></span><o:p></o:p></p>
</div>
</div>
</blockquote>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<p class="MsoNormal">Looks good.  Can we add a comment saying that objects of this class should never be allocated on the heap or something?<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><br>
<br>
<o:p></o:p></p>
<div>
<div>
<p class="MsoNormal"><span lang="EN-US" style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D"> </span><o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><span lang="EN-US" style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D">Best regards,</span><o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><span lang="EN-US" style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D">  Martin</span><o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><span lang="EN-US"> </span><o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><b><span lang="EN-US" style="font-size:10.0pt;font-family:"Tahoma","sans-serif"">From:</span></b><span class="apple-converted-space"><span lang="EN-US" style="font-size:10.0pt;font-family:"Tahoma","sans-serif""> </span></span><span lang="EN-US" style="font-size:10.0pt;font-family:"Tahoma","sans-serif"">Volker
 Simonis [<a href="mailto:volker.simonis@gmail.com">mailto:volker.simonis@gmail.com</a>]<span class="apple-converted-space"> </span><br>
<b>Sent:</b><span class="apple-converted-space"> </span>Mittwoch, 7. Oktober 2015 21:28<br>
<b>To:</b><span class="apple-converted-space"> </span>Doerr, Martin<br>
<b>Cc:</b><span class="apple-converted-space"> </span>Mikael Gerdin; Christian Thalinger; hotspot compiler<br>
<b>Subject:</b><span class="apple-converted-space"> </span>Re: RFR(S): 8138890: C1: Ambiguous operator delete</span><o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"> <o:p></o:p></p>
</div>
<div>
<p class="MsoNormal">Hi Martin,<o:p></o:p></p>
</div>
<div>
<div>
<p class="MsoNormal"> <o:p></o:p></p>
</div>
</div>
<div>
<div>
<p class="MsoNormal">as the new/delete operators in StackObj are private (I missed that before) they shouldn't be visible in LIRGenerator. So this is probably yet another xlC bug :(<o:p></o:p></p>
</div>
</div>
<div>
<div>
<p class="MsoNormal">On the other hand the new/delete operators in CompilationResourceObject are public and are inherited by LIRGenerator. So if we only want to generate LIRGenerator instances on the stack, your change is good, because it ensures this. And
 in that case we surely don't need an implementation. <o:p></o:p></p>
</div>
</div>
<div>
<div>
<p class="MsoNormal"> <o:p></o:p></p>
</div>
</div>
<div>
<div>
<p class="MsoNormal">So thumbs up from me!<o:p></o:p></p>
</div>
</div>
<div>
<div>
<p class="MsoNormal"> <o:p></o:p></p>
</div>
</div>
<div>
<div>
<p class="MsoNormal">Volker<o:p></o:p></p>
</div>
</div>
<div>
<div>
<p class="MsoNormal"><br>
<br>
On Wednesday, October 7, 2015, Doerr, Martin <<a href="mailto:martin.doerr@sap.com"><span style="color:purple">martin.doerr@sap.com</span></a>> wrote:<o:p></o:p></p>
</div>
<div>
<div>
<div>
<p class="MsoNormal"><span lang="EN-US" style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D">Hi Volker,</span><o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><span lang="EN-US" style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D"> </span><o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><span lang="EN-US" style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D">the C1 classes we are talking about should never get instantiated by operator new.</span><o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><span lang="EN-US" style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D">A typical way to establish this is to make the new operators private.</span><o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><span lang="EN-US" style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D"> </span><o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><span lang="EN-US" style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D">I don’t really care if the delete operators are public or private because if the new operator is never used, how can the delete operator get used?</span><o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><span lang="EN-US" style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D">It may be more beautiful to declare them as private as well. Only in the case Götz has showed, some Compilers reject the private delete operators.</span><o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><span lang="EN-US" style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D"> </span><o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><span lang="EN-US" style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D">Best regards,</span><o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><span lang="EN-US" style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D">Martin</span><o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><span lang="EN-US" style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D"> </span><o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><span lang="EN-US"> </span><o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><b><span lang="EN-US" style="font-size:10.0pt;font-family:"Tahoma","sans-serif"">From:</span></b><span class="apple-converted-space"><span lang="EN-US" style="font-size:10.0pt;font-family:"Tahoma","sans-serif""> </span></span><span lang="EN-US" style="font-size:10.0pt;font-family:"Tahoma","sans-serif"">Volker
 Simonis [<a href="mailto:volker.simonis@gmail.com">mailto:volker.simonis@gmail.com</a>]<span class="apple-converted-space"> </span><br>
<b>Sent:</b><span class="apple-converted-space"> </span>Mittwoch, 7. Oktober 2015 17:57<br>
<b>To:</b><span class="apple-converted-space"> </span>Mikael Gerdin<br>
<b>Cc:</b><span class="apple-converted-space"> </span>Doerr, Martin; Christian Thalinger; hotspot compiler<br>
<b>Subject:</b><span class="apple-converted-space"> </span>Re: RFR(S): 8138890: C1: Ambiguous operator delete</span><o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"> <o:p></o:p></p>
</div>
<div>
<div>
<div>
<div>
<div>
<p class="MsoNormal" style="margin-bottom:12.0pt">Hi Martin,<o:p></o:p></p>
</div>
<p class="MsoNormal" style="margin-bottom:12.0pt">we have:<br>
class LIRGenerator: public InstructionVisitor, public BlockClosure<br>
<br>
and:<br>
<br>
class BlockClosure: public CompilationResourceObj<span class="apple-converted-space"> </span><br>
<br>
class CompilationResourceObj ALLOCATION_SUPER_CLASS_SPEC {<br>
 public:<br>
  void* operator new(size_t size) throw() { return Compilation::current()->arena()->Amalloc(size); }<br>
  void* operator new(size_t size, Arena* arena) throw() {<br>
    return arena->Amalloc(size);<br>
  }<br>
  void  operator delete(void* p) {} // nothing to do<br>
};<br>
<br>
class InstructionVisitor: public StackObj<span class="apple-converted-space"> </span><br>
<br>
class StackObj ALLOCATION_SUPER_CLASS_SPEC {<br>
 private:<br>
  void* operator new(size_t size) throw();<br>
  void* operator new [](size_t size) throw();<br>
#ifdef __IBMCPP__<br>
 public:<br>
#endif<br>
  void  operator delete(void* p);<br>
  void  operator delete [](void* p);<o:p></o:p></p>
</div>
<p class="MsoNormal" style="margin-bottom:12.0pt">Now you declare new "new()" and "delete()" operators in the LIRGenerator which will actually hide the corresponding operators from the base classes. You also provide no implementation for the new operators in
 LIRGenerator. So which new/delete operators will be actually used for allocating new LIRGenerator instances?<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal" style="margin-bottom:12.0pt">OK, wait. As far as I can see, LIRGenerator is never dynamically allocated, right? In that case it should be a StackObj and you could probably solve the problem with "using" directives (e.g. using StackObj::operator
 new, ...). Have you tried that?<o:p></o:p></p>
</div>
<div>
<div>
<p class="MsoNormal">Regards,<o:p></o:p></p>
</div>
</div>
<div>
<div>
<p class="MsoNormal">Volker<o:p></o:p></p>
</div>
</div>
<div>
<p class="MsoNormal"> <o:p></o:p></p>
</div>
</div>
</div>
<div>
<div>
<p class="MsoNormal"> <o:p></o:p></p>
</div>
<div>
<div>
<p class="MsoNormal"><span lang="EN-US">On Wed, Oct 7, 2015 at 4:55 PM, Mikael Gerdin <</span><a href="mailto:mikael.gerdin@oracle.com"><span lang="EN-US">mikael.gerdin@oracle.com</span></a><span lang="EN-US">> wrote:<o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal">On 2015-10-07 16:17, Doerr, Martin wrote:<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal">Hi,<br>
<br>
that’s a good question J<br>
<br>
I can only remember that there were problems with some old compilers.<br>
<br>
Anyway, xlC 12.1 can deal with the private delete operators.<o:p></o:p></p>
</div>
<p class="MsoNormal" style="margin-bottom:12.0pt"><br>
If that's the case, can we also get rid of the workaround in allocation.hpp?<br>
<br>
Thanks<br>
/Mikael<o:p></o:p></p>
<p class="MsoNormal" style="margin-bottom:12.0pt"><br>
<span lang="EN-US">Here’s the new webrev:<br>
<br>
</span><a href="http://cr.openjdk.java.net/~mdoerr/8138890_c1_ambiguous_delete/webrev.01" target="_blank"><span lang="EN-US" style="color:purple">http://cr.openjdk.java.net/~mdoerr/8138890_c1_ambiguous_delete/webrev.01</span></a><span lang="EN-US"><br>
<br>
Best regards,<br>
<br>
   Martin<br>
<br>
*From:*Christian Thalinger [</span><a href="mailto:christian.thalinger@oracle.com"><span lang="EN-US">mailto:christian.thalinger@oracle.com</span></a><span lang="EN-US">]<br>
*Sent:* Mittwoch, 7. Oktober 2015 03:32<br>
*To:* Doerr, Martin<br>
*Cc:* hotspot compiler<br>
*Subject:* Re: RFR(S): 8138890: C1: Ambiguous operator delete<br>
<br>
    On Oct 6, 2015, at 3:56 AM, Doerr, Martin <</span><a href="mailto:martin.doerr@sap.com%0b %20 %20%3cmailto:martin.doerr@sap.com"><span lang="EN-US">martin.doerr@sap.com<br>
    <mailto:martin.doerr@sap.com</span></a><span lang="EN-US">>> wrote:<br>
<br>
    Hi,<br>
<br>
    xlC on AIX rejects to compile LIRGenerator and<br>
    RangeCheckEliminator::Verification due to ambiguous operator delete<br>
    which gets inherited from multiple base classes.<br>
<br>
    </span>This change is a prerequisite for our C1 on PPC64 contribution.<br>
<br>
    Webrev is here:<br>
<br>
   <span class="apple-converted-space"> </span><a href="http://cr.openjdk.java.net/~mdoerr/8138890_c1_ambiguous_delete/webrev.00" target="_blank"><span style="color:purple">http://cr.openjdk.java.net/~mdoerr/8138890_c1_ambiguous_delete/webrev.00</span></a><br>
<br>
Let me ask my question here:  why do you need the delete methods to be<br>
public on AIX?<br>
<br>
<br>
<br>
Please review this change.  I need a sponsor, please.<br>
<br>
Best regards,<br>
<br>
   Martin<o:p></o:p></p>
</div>
</div>
</div>
</div>
</div>
</div>
</div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
</body>
</html>