[PATCH] D13848: [Polly] Avoid unnecessay .s2a write access when used only in PHIs

[Tobias Grosser via llvm-commits]

grosser accepted this revision.
grosser added a comment.
This revision is now accepted and ready to land.

Hi Michael,

this now LGTM. I just had one comment on how to possibly make this code more understandable.

Best,
Tobias


================
Comment at: lib/Analysis/ScopInfo.cpp:3277
@@ +3276,3 @@
+    // PHIs are handled like any escaping SCALAR. Otherwise, as if the PHI
+    // belongs to the the scop region.
+    bool IsExitNodePHI = isa<PHINode>(UI) && UI->getParent() == R->getExit();
----------------
is A single exiting block

What are 'node PHIs'? Do you mean PHI nodes?

The last sentence is incomplete. It does not contain a verb in the main phrase.

It also took me a little while to understand this condition. I have the feeling moving this check into its own condition and elaborating a little more on why we bail out may make this more understandable. Does the following explain correctly what is going on? If it does, maybe you want to add some of this information to your final commit.

```
    // Check for PHI nodes in the region exit and skip them, if they will be modeled
    // as PHI nodes.
    // 
    // PHI nodes in the region exit that have more than two incoming edges need to
    // be modeled as PHI-Nodes to correctly model the fact that depending on the
    // control flow a different value will be assigned to the PHI node. In case this
    // is the case, there is no need to create an additional normal scalar dependence.
    // Hence bail out, before we register an "out-of-region" use for this definition.
    if (isa<PHINode>(UI) && UI->getParent() == R->getExit() &&                    
        !R->getExitingBlock())                                                    
      continue;                                                                   
                                                                                  
    // Check whether or not the use is in the SCoP.                               
    if (!R->contains(UseParent)) {                                                
      AnyCrossStmtUse = true;                                                     
      continue;                                                                   
    } 
```


http://reviews.llvm.org/D13848