On Thu, Jul 2, 2015 at 1:44 PM, Tobias Grosser <tob...@grosser.es> wrote: >> If you git log grep for this commit, you would see that this patch >> reverts this "typo" introduced in a very large patch. > > > Sure. The corresponding change was: > > - gimple_stmt_iterator psi = gsi_for_stmt (use_stmt); > + gphi_iterator psi = gsi_start_phis (gimple_bb (use_stmt)); > > Now this commit is not a pure revert. Instead of falling back to
IMO the patch restores the semantics, so it is a revert to some syntax changes: in the past we had this: > - gimple_stmt_iterator psi = gsi_for_stmt (use_stmt); that is get me an iterator pointing on the use_stmt. After our patch we get the same semantics back (modulo some change in function names, c++-ification, etc.) gphi *phi = dyn_cast <gphi *> (use_stmt) gphi_iterator psi = gsi_for_phi (phi); that is again an iterator pointing on the use_stmt. The patch at r217787 was incorrectly initializing the iterator to point at the beginning of the phi nodes in the BB of the use_stmt: > + gphi_iterator psi = gsi_start_phis (gimple_bb (use_stmt)); This makes no sense, as then we would start processing a random phi node and would fail to insert an array for a virtual phi node. Sebastian > gsi_for_stmt, we now use gsi_for_phi(phi) and also somehow modify the > condition above. I assume this is still correct, but as I am a little out of > graphite, it would really help to explain (in two sentences in the commit > message) why the previous change was wrong and how the behavior is now > different. Something like: > > "After this patch we start to iterate at the very first phi node, > whereas before this applied we skipped the PHI nodes and started iterating > at the first actual instruciton." > > Thanks, > Tobias
