FYI: rework previous fix
Roland Westrelin
rwestrel at redhat.com
Wed Sep 27 15:45:27 UTC 2017
> This seems to revert our differences around has_only_data_users()
> against upstream in favor of this one-liner, right?
Not quite. This below is new:
@@ -2520,12 +2495,6 @@
Node* n_loop_head = n_loop->_head;
if (n_loop_head->is_Loop()) {
- int alias = phase->C->get_alias_index(adr_type());
- Node* mem = find_mem_phi(n_loop_head, alias, phase->C);
- if (mem == NULL) {
- mem = in(Memory);
- }
-
LoopNode* loop = n_loop_head->as_Loop();
if (n_loop_head->is_CountedLoop() && n_loop_head->as_CountedLoop()->is_main_loop()) {
LoopNode* res = try_move_before_pre_loop(n_loop_head->in(LoopNode::EntryControl), val_ctrl, phase);
> 878 } else if (in->Opcode() == Op_CMoveP || in->Opcode() == Op_CMoveN) {
That's a fix for the verification pass so it's unrelated.
> Make sure you have descriptive commit message!
Won't history all be lost when things are upstreamed so we should rather
put comments?
Here is the explanation:
When a WB is moved out of loop, we must hook its memory edges out of the
loop and that's done by finding the memory Phi for that memory slice at
the loop head. The bug is that the WB can be connected to a Phi that is
later optimized out and then the WB is no longer connected correctly to
the memory graph. That happens when the loop has a memory Phi for bottom
memory and a memory Phi for the WB memory slice. The Phi on the WB
memory slice may only have data uses. If the data uses go away, then the
Phi goes away as well. The current code finds the Phi by looking at all
Phis for the loop and picking the most specific one so it could pick the
wrong phi. The previous fix was to locate phis with only data uses and
move uses off that phi to the bottom phi. The fix now, is to not find
the phi at the loop by looking at all loop phis but instead to start
from the WB in the loop, follow memory edges until the loop and use the
phi that is there which I believe is safe.
Roland.
More information about the shenandoah-dev
mailing list