stan/jdk-24
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

8206163: AArch64: incorrect code generation for StoreCM

Browse Source 
StoreCM may require planting a StoreStore barrier

Reviewed-by: aph, zyao, roland
This commit is contained in:
Andrew Dinn 2018-07-09 09:38:11 +01:00
parent cb9a168f8b
commit d5c20788ae
2 changed files with 23 additions and 57 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

61
src/hotspot/cpu/aarch64/aarch64.ad
View File

@ -1471,7 +1471,7 @@ source %{
// Ctl+Mem to a StoreB node (which does the actual card mark). // Ctl+Mem to a StoreB node (which does the actual card mark).
// //
// n.b. a StoreCM node will only appear in this configuration when // n.b. a StoreCM node will only appear in this configuration when
// using CMS. StoreCM differs from a normal card mark write (StoreB) // using CMS or G1. StoreCM differs from a normal card mark write (StoreB)
// because it implies a requirement to order visibility of the card // because it implies a requirement to order visibility of the card
// mark (StoreCM) relative to the object put (StoreP/N) using a // mark (StoreCM) relative to the object put (StoreP/N) using a
// StoreStore memory barrier (arguably this ought to be represented // StoreStore memory barrier (arguably this ought to be represented
@ -1481,16 +1481,12 @@ source %{
// the sequence // the sequence
// //
// dmb ishst // dmb ishst
// stlrb // strb
// //
// However, in the case of a volatile put if we can recognise this // However, when using G1 or CMS with conditional card marking (as
// configuration and plant an stlr for the object write then we can // we shall see) we don't need to insert the dmb when translating
// omit the dmb and just plant an strb since visibility of the stlr // StoreCM because there is already an intervening StoreLoad barrier
// is ordered before visibility of subsequent stores. StoreCM nodes // between it and the StoreP/N.
// also arise when using G1 or using CMS with conditional card
// marking. In these cases (as we shall see) we don't need to insert
// the dmb when translating StoreCM because there is already an
// intervening StoreLoad barrier between it and the StoreP/N.
// //
// It is also possible to perform the card mark conditionally on it // It is also possible to perform the card mark conditionally on it
// currently being unmarked in which case the volatile put graph // currently being unmarked in which case the volatile put graph
@ -2868,50 +2864,17 @@ bool unnecessary_storestore(const Node *storecm)
{ {
assert(storecm->Opcode() == Op_StoreCM, "expecting a StoreCM"); assert(storecm->Opcode() == Op_StoreCM, "expecting a StoreCM");
// we only ever need to generate a dmb ishst between an object put // we need to generate a dmb ishst between an object put and the
// and the associated card mark when we are using CMS without // associated card mark when we are using CMS without conditional
// conditional card marking // card marking
if (!UseConcMarkSweepGC || UseCondCardMark) { if (UseConcMarkSweepGC && !UseCondCardMark) {
return true;
}
// if we are implementing volatile puts using barriers then the
// object put is an str so we must insert the dmb ishst
if (UseBarriersForVolatile) {
return false; return false;
} }
// we can omit the dmb ishst if this StoreCM is part of a volatile // a storestore is unnecesary in all other cases
// put because in thta case the put will be implemented by stlr
//
// we need to check for a normal subgraph feeding this StoreCM.
// that means the StoreCM must be fed Memory from a leading membar,
// either a MemBarRelease or its dependent MemBarCPUOrder, and the
// leading membar must be part of a normal subgraph
Node *x = storecm->in(StoreNode::Memory); return true;
if (!x->is_Proj()) {
return false;
}
x = x->in(0);
if (!x->is_MemBar()) {
return false;
}
MemBarNode *leading = x->as_MemBar();
// reject invalid candidates
if (!leading_membar(leading)) {
return false;
}
// we can omit the StoreStore if it is the head of a normal subgraph
return (leading_to_normal(leading) != NULL);
} }

19
test/hotspot/jtreg/compiler/c2/aarch64/TestVolatiles.java
View File

@ -34,7 +34,7 @@
* TestUnsafeVolatileCAS} * TestUnsafeVolatileCAS}
* and <testtype> in {G1, * and <testtype> in {G1,
* CMS, * CMS,
* CMSCondCardMark, * CMSCondMark,
* Serial, * Serial,
* Parallel} * Parallel}
*/ */
@ -287,7 +287,7 @@ public class TestVolatiles {
"ret" "ret"
}; };
break; break;
case "CMSCondCardMark": case "CMSCondMark":
// a card mark volatile barrier should be generated // a card mark volatile barrier should be generated
// before the card mark strb from the StoreCM and the // before the card mark strb from the StoreCM and the
// storestore barrier from the StoreCM should be elided // storestore barrier from the StoreCM should be elided
@ -305,11 +305,13 @@ public class TestVolatiles {
case "CMS": case "CMS":
// a volatile card mark membar should not be generated // a volatile card mark membar should not be generated
// before the card mark strb from the StoreCM and the // before the card mark strb from the StoreCM and the
// storestore barrier from the StoreCM should be elided // storestore barrier from the StoreCM should be
// generated as "dmb ishst"
matches = new String[] { matches = new String[] {
"membar_release (elided)", "membar_release (elided)",
"stlrw", "stlrw",
"storestore (elided)", "storestore",
"dmb ishst",
"strb", "strb",
"membar_volatile (elided)", "membar_volatile (elided)",
"ret" "ret"
@ -344,7 +346,7 @@ public class TestVolatiles {
"ret" "ret"
}; };
break; break;
case "CMSCondCardMark": case "CMSCondMark":
// a card mark volatile barrier should be generated // a card mark volatile barrier should be generated
// before the card mark strb from the StoreCM and the // before the card mark strb from the StoreCM and the
// storestore barrier from the StoreCM should be elided // storestore barrier from the StoreCM should be elided
@ -443,7 +445,7 @@ public class TestVolatiles {
"ret" "ret"
}; };
break; break;
case "CMSCondCardMark": case "CMSCondMark":
// a card mark volatile barrier should be generated // a card mark volatile barrier should be generated
// before the card mark strb from the StoreCM and the // before the card mark strb from the StoreCM and the
// storestore barrier from the StoreCM should be elided // storestore barrier from the StoreCM should be elided
@ -465,7 +467,8 @@ public class TestVolatiles {
matches = new String[] { matches = new String[] {
"membar_release (elided)", "membar_release (elided)",
"cmpxchgw_acq", "cmpxchgw_acq",
"storestore (elided)", "storestore",
"dmb ishst",
"strb", "strb",
"membar_acquire (elided)", "membar_acquire (elided)",
"ret" "ret"
@ -500,7 +503,7 @@ public class TestVolatiles {
"ret" "ret"
}; };
break; break;
case "CMSCondCardMark": case "CMSCondMark":
// a card mark volatile barrier should be generated // a card mark volatile barrier should be generated
// before the card mark strb from the StoreCM and the // before the card mark strb from the StoreCM and the
// storestore barrier from the StoreCM should be elided // storestore barrier from the StoreCM should be elided