RFR(S): 8238723: yank_alloc_node must remove membar

[Vladimir Ivanov]

> You found the answer I was looking for.
> 
> So the addPs should die when I eliminate the InitalizeNode, and the I 
> remove the membar, and then things should be ok.
Not sure you can leave AddPs just hanging around. There should be a GVN 
pass to prune them, but yank_alloc_node() is called right after 
yank_initalize_node(). So, if there are stale AddPs left, I expect 
they'll be enumerated as uses of result projection and 
isa_MemBarStoreStore() will assert:

+  if (_resproj != NULL) {
+    for (DUIterator_Fast imax, i = _resproj->fast_outs(imax); i < imax; 
i++) {
+      Node* use = _resproj->fast_out(i);
+      use->isa_MemBarStoreStore()->remove(&this->igvn());
+      --imax;
+      --i; // back up iterator
+    }
+    assert(_resproj->outcnt() == 0, "all uses must be deleted");
+    _igvn.remove_dead_node(_resproj);
+  }

After looking at PhaseMacroExpand::process_users_of_allocation() more 
closely, I think it should be fine to remove everything at once when 
DUIterator_Fast is used.

Best regards,
Vladimir Ivanov


> 
> Agree?
> 
> // Nils
> 
> 
> On 2020-02-18 14:35, Vladimir Ivanov wrote:
>>
>>> What are the cases when AddPs on raw oop are present? Is it safe to 
>>> remove Allocate node in such cases?
>>
>> I found the answer: AddPs correspond to initializing stores captured 
>> by Initialize node. So, it should be fine to remove them the way you do.
>>
>> But now I'm curious whether elimination order (Initialize vs AddPs) 
>> matters or not. PhaseMacroExpand::process_users_of_allocation() has 
>> the following comment [1]:
>>
>>     // First disconnect stores captured by Initialize node.
>>     // If Initialize node is eliminated first in the following code,
>>     // it will kill such stores and DUIterator_Last will assert.
>>
>> Best regards,
>> Vladimir Ivanov
>>
>> [1] 
>> http://hg.openjdk.java.net/jdk/jdk/file/tip/src/hotspot/share/opto/macro.cpp#l1028 
>>