On 15/04/15 21:18, James Greenhalgh wrote: > On Wed, Apr 15, 2015 at 11:45:36AM +0100, Kugan wrote: >>> There are two ways I see that we could clean things up, both of which >>> require some reworking of your patch. >>> >>> Either we remove my check above and teach the RTX costs how to properly >>> cost vector operations, or we fix my check to catch all vector RTX >>> and add the special cases for the small subset of things we understand >>> up there. >>> >>> The correct approach in the long term is to fix the RTX costs to correctly >>> understand vector operations, so I'd much prefer to see a patch along >>> these lines, though I appreciate that is a substantially more invasive >>> piece of work. >>> >> >> >> I agree that rtx cost for vector is not handled right now. We might not >> be able to completely separate as Kyrill suggested. We still need the >> vector SET with VOIDmode to be handled inline. This patch is that part. >> We can work on the others as a separate function, if you prefer that. I >> am happy to look this as a separate patch. > > My point is that adding your patch while keeping the logic at the top > which claims to catch ALL vector operations makes for less readable > code. > > At the very least you'll need to update this comment: > > /* TODO: The cost infrastructure currently does not handle > vector operations. Assume that all vector operations > are equally expensive. */ > > to make it clear that this doesn't catch vector set operations. > > But fixing the comment doesn't improve the messy code so I'd certainly > prefer to see one of the other approaches which have been discussed.
I see your point. Let me work on this based on your suggestions above. Thanks, Kugan