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: PR61140: check the phi is unique in value_replacement


On Sat, 10 May 2014, Andrew Pinski wrote:

On Sat, May 10, 2014 at 3:53 PM, Marc Glisse <marc.glisse@inria.fr> wrote:
Hello,

in my recent phiopt patch enhancing value_replacement to optimize
x!=0?x+y:y, I forgot to check that there is no other PHI (not sure how I
managed to miss that since I copy-pasted the line just below the test).

If there are other phi nodes (with different arguments for those 2
branches), it would be possible to replace the phi argument and stop there
(as value_replacement does for its other transformation). However, I am
chosing to punt. The cost analysis would be different, and I wrote the
transformation assuming that this single-phi test was already done higher in
the function.

I think we should have some good cost analysis because for this
testcase, we should be able to get only one conditional move but right
now with punting we don't.

That's true. But note that the transformation is already very limited (gives up if there is a second statement in the middle bb, even a simple cast), so I would like to first quickly get the wrong-code regression out of the way, and we can make improvements afterwards (though we can of course start discussing them now).

It seems like if there is only 1 extra non-singleton phi (in addition to the one we are transforming) and the target supports conditional move for this type and the direct branch has proba < 50%, with the other restrictions already in place, we could go ahead. How does that sound? Not too specialized? If there are many phis, conditional moves are out, the branch will stay, and unless the edge to the operation has a very high proba, it doesn't seem like a good idea to pull the operation out of the branch.

--
Marc Glisse


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