[aarch64-port-dev ] RFR: Bulk integration of Shenandoah 2018-05-15
Roman Kennke
rkennke at redhat.com
Wed Jun 6 08:21:26 UTC 2018
Hello Andrew,
are you ok with Aleksey's explanations? Can the changes go in?
Thanks, Roman
> On 06/04/2018 06:30 PM, Andrew Haley wrote:
>> On 05/31/2018 09:37 AM, Roman Kennke wrote:
>>> Has anybody else gotten to review this in the meantime? Is it ok to go?
>>
>> I had a look. I didn't scan the Shenadoah changes, but the shared code
>> changes mostly look innocuous. I didn't look at the tests either.
>
> Thank you!
>
> Rearranging replies a bit:
>
> === Real changes
>
> --- This change is safe: exclude_fp is false by default, and it is true on some Shenandoah-specific
> paths. We can guard the "if (exclude_fp)" with additional UseShenandoahGC, but it looks like overkill.
>
>> +++ b/src/share/vm/opto/lcm.cpp
>> @@ -638,9 +638,12 @@
>>
>> //------------------------------add_call_kills-------------------------------------
>> // helper function that adds caller save registers to MachProjNode
>> -static void add_call_kills(MachProjNode *proj, RegMask& regs, const char* save_policy, bool exclude_soe) {
>> +static void add_call_kills(MachProjNode *proj, RegMask& regs, const char* save_policy, bool exclude_soe, bool exclude_fp) {
>> // Fill in the kill mask for the call
>> for( OptoReg::Name r = OptoReg::Name(0); r < _last_Mach_Reg; r=OptoReg::add(r,1) ) {
>> + if (exclude_fp && (register_save_type[r] == Op_RegF || register_save_type[r] == Op_RegD)) {
>> + continue;
>> + }
>>
>> @@ -930,7 +938,7 @@
>> map_node_to_block(proj, block);
>> block->insert_node(proj, phi_cnt++);
>>
>> - add_call_kills(proj, regs, _matcher._c_reg_save_policy, false);
>> + add_call_kills(proj, regs, _matcher._c_reg_save_policy, false, false);
>> }
>
>
> === Reverts of Shenandoah-specific code
>
> --- These blocks, including all these has_only_data_users(), were added long ago for Shenandoah, and
> this is the revert to upstream state -- not sure why weren't they protected with UseShenandoahGC
>
>> diff --git a/src/share/vm/opto/cfgnode.cpp b/src/share/vm/opto/cfgnode.cpp
>> --- a/src/share/vm/opto/cfgnode.cpp
>> +++ b/src/share/vm/opto/cfgnode.cpp
>> @@ -1153,20 +1153,6 @@
>> if (id != NULL) return id;
>> }
>>
>> - if (phase->is_IterGVN()) {
>> - // A memory Phi could only have data inputs if that Phi was input
>> - // to a MergeMem and MergeMemNode::Ideal() found that it's
>> - // equivalent to the base memory Phi. If data uses are removed,
>> - // the Phi will be removed as well. If one of the Phi inputs has
>> - // changed in the meantime (shenandoah write barrier moved out of
>> - // loop for instance), that input is disconnected from the memory
>> - // graph.
>> - Node* other_phi = has_only_data_users();
>> - if (other_phi != NULL) {
>> - return other_phi;
>> - }
>> - }
>> -
>> return this; // No identity
>> }
>> @@ -2122,30 +2108,6 @@
>> _dest_bci == ((JumpProjNode&)n)._dest_bci;
>> }
>>
>> -PhiNode* PhiNode::has_only_data_users() const {
>> - if (!UseShenandoahGC || bottom_type() != Type::MEMORY || adr_type() == TypePtr::BOTTOM || outcnt() == 0) {
>> - return NULL;
>> - }
>> - for (DUIterator_Fast imax, i = fast_outs(imax); i < imax; i++) {
>> - Node* u = fast_out(i);
>> - if (u->Opcode() != Op_ShenandoahReadBarrier) {
>> - return NULL;
>> - }
>> - }
>> - Node* r = in(0);
>> - if (r == NULL) {
>> - return NULL;
>> - }
>> - for (DUIterator_Fast imax, i = r->fast_outs(imax); i < imax; i++) {
>> - Node* u = r->fast_out(i);
>> - if (u != this && u->is_Phi() && u->bottom_type() == Type::MEMORY &&
>> - u->adr_type() == TypePtr::BOTTOM) {
>> - return u->as_Phi();
>> - }
>> - }
>> - return NULL;
>> -}
>> -
>> #ifndef PRODUCT
>> void JumpProjNode::dump_spec(outputStream *st) const {
>> ProjNode::dump_spec(st);
>> diff --git a/src/share/vm/opto/cfgnode.hpp b/src/share/vm/opto/cfgnode.hpp
>> --- a/src/share/vm/opto/cfgnode.hpp
>> +++ b/src/share/vm/opto/cfgnode.hpp
>> @@ -220,8 +220,6 @@
>> #else //ASSERT
>> void verify_adr_type(bool recursive = false) const {}
>> #endif //ASSERT
>> -
>> - PhiNode* has_only_data_users() const;
>> };
>> //------------------------------GotoNode---------------------------------------
>> diff --git a/src/share/vm/opto/compile.cpp b/src/share/vm/opto/compile.cpp
>> --- a/src/share/vm/opto/compile.cpp
>> +++ b/src/share/vm/opto/compile.cpp
>> @@ -416,9 +416,6 @@
>> record_for_igvn(n->fast_out(i));
>> }
>> }
>> - if (n->is_Phi() && n->as_Phi()->has_only_data_users()) {
>> - record_for_igvn(n);
>> - }
>> }
>> // Remove useless macro and predicate opaq nodes
>> for (int i = C->macro_count()-1; i >= 0; i--) {
>> @@ -3149,7 +3146,6 @@
>> n->subsume_by(unique_in, this);
>> }
>> }
>> - assert(!n->as_Phi()->has_only_data_users(), "memory Phi has no memory use");
>> break;
>>
>> #endif
>> diff --git a/src/share/vm/opto/node.cpp b/src/share/vm/opto/node.cpp
>> --- a/src/share/vm/opto/node.cpp
>> +++ b/src/share/vm/opto/node.cpp
>> @@ -1406,8 +1406,6 @@
>> igvn->add_users_to_worklist( n );
>> } else if (n->Opcode() == Op_AddP && CallLeafNode::has_only_g1_wb_pre_uses(n)) {
>> igvn->add_users_to_worklist(n);
>> - } else if (n->is_Phi() && n->as_Phi()->has_only_data_users()) {
>> - igvn->_worklist.push(n);
>> }
>> }
>> }
>> diff --git a/src/share/vm/opto/phaseX.cpp b/src/share/vm/opto/phaseX.cpp
>> --- a/src/share/vm/opto/phaseX.cpp
>> +++ b/src/share/vm/opto/phaseX.cpp
>> @@ -1300,8 +1300,6 @@
>> _worklist.push(in);
>> } else if (in->Opcode() == Op_AddP && CallLeafNode::has_only_g1_wb_pre_uses(in)) {
>> add_users_to_worklist(in);
>> - } else if (in->is_Phi() && in->as_Phi()->has_only_data_users()) {
>> - _worklist.push(in);
>> }
>> if (ReduceFieldZeroing && dead->is_Load() && i == MemNode::Memory &&
>> in->is_Proj() && in->in(0) != NULL && in->in(0)->is_Initialize()) {
>> @@ -1978,9 +1976,6 @@
>> if (old->Opcode() == Op_AddP && CallLeafNode::has_only_g1_wb_pre_uses(old)) {
>> igvn->add_users_to_worklist(old);
>> }
>> - if (old->is_Phi() && old->as_Phi()->has_only_data_users()) {
>> - igvn->_worklist.push(old);
>> - }
>> }
>>
>> }
>>
>
> --- This is the revert to upstream state, see:
> http://hg.openjdk.java.net/jdk8u/jdk8u/hotspot/file/64fe89b445cd/src/share/vm/opto/memnode.cpp#l4379
>
>> diff --git a/src/share/vm/opto/memnode.cpp b/src/share/vm/opto/memnode.cpp
>> --- a/src/share/vm/opto/memnode.cpp
>> +++ b/src/share/vm/opto/memnode.cpp
>> @@ -4229,11 +4229,7 @@
>> // Warning: Do not combine this "if" with the previous "if"
>> // A memory slice might have be be rewritten even if it is semantically
>> // unchanged, if the base_memory value has changed.
>> - if (can_reshape) {
>> - set_req_X(i, new_in, phase->is_IterGVN());
>> - } else {
>> - set_req(i, new_in);
>> - }
>> + set_req(i, new_in);
>> progress = this; // Report progress
>> }
>> }
>
>
>
> Thanks,
> -Aleksey
>
More information about the aarch64-port-dev
mailing list