On 21/10/2015 1:53 PM, Carsten Varming wrote:
Dear David,
In this case dummytype is the result of a typedef. "typedef int*
dummytype; volatile dummytype * dummy" is the same as "typedef int*
dummytype; dummytype volatile * dummy". Nevertheless, I always recommend
sticking to postfix unary type operators in macros to minimize confusion
with substitution.
The role of the typedef was tickling something in my memory :) Yes you
are right - the typedef solves this problem. Which means that the
existing is correct and Dan's new macro is not needed either.
Thanks,
David
-----
Carsten
On Tue, Oct 20, 2015 at 8:39 PM, David Holmes <[email protected]
<mailto:[email protected]>> wrote:
On 21/10/2015 1:37 PM, Daniel D. Daugherty wrote:
On 10/20/15, 9:18 PM, David Holmes wrote:
<trimming>
On 21/10/2015 12:44 PM, Daniel D. Daugherty wrote:
On 10/20/15, 8:15 PM, David Holmes wrote:
On 21/10/2015 12:51 AM, Daniel D. Daugherty wrote:
On 10/20/15, 1:53 AM, David Holmes wrote:
src/share/vm/runtime/vmStructs.cpp
Can you not just define volatile_static_field?
Yes, I went that way originally and then I
changed my mind to
avoid colliding with the other format. See below.
Why does the ptr aspect need to come into
it? Also "static pointer
volatile field" sounds really odd, it is a
static, volatile field
that
happens to be a pointer-type.
It's meant to be odd because it follows the
actual decl:
static ObjectMonitor * volatile gBlockList;
So "static pointer volatile field" is exactly
what I have:
static ObjectMonitor * volatile gBlockList;
=> (static ObjectMonitor *) volatile gBlockList;
i.e. I have a static ObjectMonitor pointer that
is volatile
Compared to these two forms:
static volatile ObjectMonitor * gBlockList;
static ObjectMonitor volatile * gBlockList;
=> static (volatile ObjectMonitor) * gBlockList;
=> static (ObjectMonitor volatile) * gBlockList;
i.e. I have a static pointer to a volatile
ObjectMonitor.
Hopefully, this makes my reasons a bit more clear...
Not really :) Yes there is a difference between a
"volatile pointer to
Foo" and "pointer to a volatile Foo", but for the
sake of vmstructs we
don't really seem to need to care about that. The
two questions are:
- is the field/variable static
- is the field/variable volatile
I'll have to politely disagree:
Here's the existing volatile non-static macro:
2743 // This macro checks the type of a volatile
VMStructEntry by
comparing pointer types
2744 #define
CHECK_VOLATILE_NONSTATIC_VM_STRUCT_ENTRY(typeName,
fieldName, type) \
2745 {typedef type dummyvtype; typeName *dummyObj =
NULL; volatile
dummyvtype* dummy = &dummyObj->fieldName; }
And here's the new static pointer volatile macro:
2751 // This macro checks the type of a static pointer
volatile
VMStructEntry by comparing pointer types,
2752 // e.g.: "static ObjectMonitor * volatile gBlockList;"
2753 #define
CHECK_STATIC_PTR_VOLATILE_VM_STRUCT_ENTRY(typeName,
fieldName, type) \
2754 {type volatile * dummy = &typeName::fieldName; }
Yes, the variable assignments are different because we
have static
versus a non-static situation, but what's more important
is where
the "volatile" is positioned.
I see your point. But I think the real problem is that there
is a bug
in the declaration of
CHECK_VOLATILE_NONSTATIC_VM_STRUCT_ENTRY that
makes it wrong when used with a pointer type. I think this:
2745 {typedef type dummyvtype; typeName *dummyObj = NULL;
volatile
dummyvtype* dummy = &dummyObj->fieldName; }
should really be:
2745 {typedef type dummyvtype; typeName *dummyObj = NULL;
dummyvtype
volatile * dummy = &dummyObj->fieldName; }
Actually, I believe these two are equivalent:
volatile dummyvtype* dummy =
dummyvtype volatile * dummy =
based on my reading of the URL that I put in the original webrev...
So it's not a bug, it's one variation of an acceptable style.
Not when dummyvtype is itself a pointer type. Consider:
volatile int* *dummy = ...;
Here dummy is a pointer to a pointer to a volatile int.
But in:
int* volatile *dummy = ...;
dummy is a pointer to a volatile pointer to an int
Cheers,
David
-----
and the static version would follow the same form. dummy is
a pointer
to a volatile field of type dummyvtype. (I'm unclear why the
dummyObj
variable is introduced though ??).
'dummyObj' is used to access the field: &dummyObj->fieldName
I wonder if Kim wants to wade in on this one :)
Dunno.
Dan
Cheers,
David
-----
In the existing volatile non-static macro, the volatile
piece is:
volatile dummyvtype* dummy = &dummyObj->fieldName;
and in the new static pointer volatile macro, the
volatile piece is:
type volatile * dummy = &typeName::fieldName;
So the CHECK_VOLATILE_NONSTATIC_XXX macro has the
"volatile" before
the data type... and the CHECK_STATIC_PTR_VOLATILE_XXX macro
has the "volatile" after the data type...
Dan
Cheers,
David
Dan
Thanks,
David
Testing: Aurora Adhoc RT-SVC nightly batch
4 inner-complex fastdebug
parallel runs for 4+ days and
600K iterations without
seeing this failure; the
experiment
is still running; final
results to be reported at the
end
of the review cycle
JPRT -testset hotspot
This fix:
- makes ObjectMonitor::gBlockList volatile
- uses
"OrderAccess::release_store_ptr(&gBlockList,
temp)" to
make sure the new block updates
_happen before_ gBlockList is
changed to refer to the new block
- add SA support for a "static pointer
volatile" field like:
static ObjectMonitor * volatile
gBlockList;
See the following link for a nice
description of what "volatile"
means in the different positions on a
variable/parameter decl line:
http://www.embedded.com/electronics-blogs/beginner-s-corner/4023801/Introduction-to-the-Volatile-Keyword
Thanks, in advance, for any comments,
questions or suggestions.
Dan
