This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] Restore previous change for gimple_phi_iterator


On 07/02/2015 09:03 PM, Sebastian Pop wrote:
On Thu, Jul 2, 2015 at 1:44 PM, Tobias Grosser <tobias@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.

Thanks. I am a little slow today. The patch looks indeed correct. Maybe you could add this explanation to the commit message and also add a test case as Ramana suggested.

Tobias


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]