RFR (M): 8168467: Use TaskEntry as task mark queue elements

[Thomas Schatzl]

Hi,

On Thu, 2017-03-09 at 16:28 -0500, Kim Barrett wrote:
> > 
> > On Mar 8, 2017, at 5:31 AM, Thomas Schatzl <thomas.schatzl at oracle.c
> > om> wrote:
> > 
> > Hi,
> > 
> > On Tue, 2017-03-07 at 19:11 -0500, Kim Barrett wrote:
> > > 
> > > > 
[...]
> > 
> > I prepared a patch for how this would look like:
> > http://cr.openjdk.java.net/~tschatzl/8168467/webrev.1_to_2/
> > http://cr.openjdk.java.net/~tschatzl/8168467/webrev.2/
> > 
> > I will probably need to add a comment now why we use an extra
> > assign method instead of the operator now :-)
> > 
> > As for the change, if you insist on having an extra assign method
> > just to avoid this pragma and decrease readability of the uses of
> > that code, let's use webrev.2 as the current version.
> I agree the assign functioning approach is kind of ugly.
> 
> But you still haven't answered my question as to whether the
> nonvolatile operator is actually needed, e.g. just use the volatile
> operator always.

I guess I simply did not understand your question (or the motivation
for your question)...

> But I finally remembered this:
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=7614
> (Sorry it took so long to dredged this up, but it's been maybe a
> dozen years since I tripped over it.)
> 
> Discussion there suggests returning volatile& is problematic, which
> seems right. The callers (GenericTaskQueue's push and push_slow) need
> to const_cast away the volatile and then cast to void the result.
> Much simpler would be an operation that didn't return volatile&
> (either a void returning volatile operator= or a named function).
> 
> This is all getting rather far afield from the change at hand though.
> I suggest going with webrev.1 and a cleanup RFE around volatile
> operator=. (I suggest no volatile operator=, instead a volatile
> assign function, and change taskqueue push to use that.)
> 
> I think StarTask and maybe the various flavors of oop are related.
> Hm, why doesn't the oop class need similar warning suppression?

I found that if CHECK_UNHANDLED_OOPS is enabled (to compile the
relevant code in oopsHierarchy.hpp), there is a global pragma to
disable this warning.


> If you agree, then webrev.1 looks good to me.
> 

Agree. Let's keep webrev.1.

Thanks,
  Thomas