RFR(S): 8138890: C1: Ambiguous operator delete
Christian Thalinger
christian.thalinger at oracle.com
Fri Oct 9 22:32:24 UTC 2015
> On Oct 9, 2015, at 9:08 AM, Doerr, Martin <martin.doerr at sap.com> wrote:
>
> Hi Christian,
>
> I have just added comments. We can also get rid of the multi-inheritance in RanceCheckEliminator::Verification.
Hmm. Why did we have it this way (not using the macro and such)?
- class Verification : public _ValueObj /*VALUE_OBJ_CLASS_SPEC*/, public BlockClosure {
+ class Verification : public BlockClosure {
>
> The new webrev is:
> http://cr.openjdk.java.net/~mdoerr/8138890_c1_ambiguous_delete/webrev.03 <http://cr.openjdk.java.net/~mdoerr/8138890_c1_ambiguous_delete/webrev.03>
> Best regards,
> Martin
>
> From: Christian Thalinger [mailto:christian.thalinger at oracle.com]
> Sent: Freitag, 9. Oktober 2015 20:35
> To: Doerr, Martin
> Cc: Volker Simonis; Lindenmaier, Goetz; Mikael Gerdin; hotspot compiler
> Subject: Re: RFR(S): 8138890: C1: Ambiguous operator delete
>
>
> On Oct 9, 2015, at 12:03 AM, Doerr, Martin <martin.doerr at sap.com <mailto:martin.doerr at sap.com>> wrote:
>
> Hi,
>
> thanks for reviewing.
> As requested by Götz, I just removed the second ‘private’.
> The new webrev is here:
> http://cr.openjdk.java.net/~mdoerr/8138890_c1_ambiguous_delete/webrev.02 <http://cr.openjdk.java.net/~mdoerr/8138890_c1_ambiguous_delete/webrev.02>
>
> Looks good. Can we add a comment saying that objects of this class should never be allocated on the heap or something?
>
>
>
> Best regards,
> Martin
>
> From: Volker Simonis [mailto:volker.simonis at gmail.com <mailto:volker.simonis at gmail.com>]
> Sent: Mittwoch, 7. Oktober 2015 21:28
> To: Doerr, Martin
> Cc: Mikael Gerdin; Christian Thalinger; hotspot compiler
> Subject: Re: RFR(S): 8138890: C1: Ambiguous operator delete
>
> Hi Martin,
>
> 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 :(
> 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.
>
> So thumbs up from me!
>
> Volker
>
>
> On Wednesday, October 7, 2015, Doerr, Martin <martin.doerr at sap.com <mailto:martin.doerr at sap.com>> wrote:
> Hi Volker,
>
> the C1 classes we are talking about should never get instantiated by operator new.
> A typical way to establish this is to make the new operators private.
>
> 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?
> 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.
>
> Best regards,
> Martin
>
>
> From: Volker Simonis [mailto:volker.simonis at gmail.com <mailto:volker.simonis at gmail.com>]
> Sent: Mittwoch, 7. Oktober 2015 17:57
> To: Mikael Gerdin
> Cc: Doerr, Martin; Christian Thalinger; hotspot compiler
> Subject: Re: RFR(S): 8138890: C1: Ambiguous operator delete
>
> Hi Martin,
>
> we have:
> class LIRGenerator: public InstructionVisitor, public BlockClosure
>
> and:
>
> class BlockClosure: public CompilationResourceObj
>
> class CompilationResourceObj ALLOCATION_SUPER_CLASS_SPEC {
> public:
> void* operator new(size_t size) throw() { return Compilation::current()->arena()->Amalloc(size); }
> void* operator new(size_t size, Arena* arena) throw() {
> return arena->Amalloc(size);
> }
> void operator delete(void* p) {} // nothing to do
> };
>
> class InstructionVisitor: public StackObj
>
> class StackObj ALLOCATION_SUPER_CLASS_SPEC {
> private:
> void* operator new(size_t size) throw();
> void* operator new [](size_t size) throw();
> #ifdef __IBMCPP__
> public:
> #endif
> void operator delete(void* p);
> void operator delete [](void* p);
>
> 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?
>
> 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?
>
> Regards,
> Volker
>
>
> On Wed, Oct 7, 2015 at 4:55 PM, Mikael Gerdin <mikael.gerdin at oracle.com <mailto:mikael.gerdin at oracle.com>> wrote:
> On 2015-10-07 16:17, Doerr, Martin wrote:
> Hi,
>
> that’s a good question J
>
> I can only remember that there were problems with some old compilers.
>
> Anyway, xlC 12.1 can deal with the private delete operators.
>
> If that's the case, can we also get rid of the workaround in allocation.hpp?
>
> Thanks
> /Mikael
>
>
> Here’s the new webrev:
>
> http://cr.openjdk.java.net/~mdoerr/8138890_c1_ambiguous_delete/webrev.01 <http://cr.openjdk.java.net/~mdoerr/8138890_c1_ambiguous_delete/webrev.01>
>
> Best regards,
>
> Martin
>
> *From:*Christian Thalinger [mailto:christian.thalinger at oracle.com <mailto:christian.thalinger at oracle.com>]
> *Sent:* Mittwoch, 7. Oktober 2015 03:32
> *To:* Doerr, Martin
> *Cc:* hotspot compiler
> *Subject:* Re: RFR(S): 8138890: C1: Ambiguous operator delete
>
> On Oct 6, 2015, at 3:56 AM, Doerr, Martin <martin.doerr at sap.com
> <mailto:martin.doerr at sap.com <mailto:martin.doerr at sap.com%0b %20 %20%3cmailto:martin.doerr at sap.com>>> wrote:
>
> Hi,
>
> xlC on AIX rejects to compile LIRGenerator and
> RangeCheckEliminator::Verification due to ambiguous operator delete
> which gets inherited from multiple base classes.
>
> This change is a prerequisite for our C1 on PPC64 contribution.
>
> Webrev is here:
>
> http://cr.openjdk.java.net/~mdoerr/8138890_c1_ambiguous_delete/webrev.00 <http://cr.openjdk.java.net/~mdoerr/8138890_c1_ambiguous_delete/webrev.00>
>
> Let me ask my question here: why do you need the delete methods to be
> public on AIX?
>
>
>
> Please review this change. I need a sponsor, please.
>
> Best regards,
>
> Martin
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/attachments/20151009/3759afe6/attachment-0001.html>
More information about the hotspot-compiler-dev
mailing list