This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [AArch64][PR65375] Fix RTX cost for vector SET
- From: Maxim Kuvyrkov <maxim dot kuvyrkov at linaro dot org>
- To: James Greenhalgh <james dot greenhalgh at arm dot com>
- Cc: Kugan <kugan dot vivekanandarajah at linaro dot org>, Kyrylo Tkachov <Kyrylo dot Tkachov at arm dot com>, "gcc-patches at gcc dot gnu dot org" <gcc-patches at gcc dot gnu dot org>, Marcus Shawcroft <Marcus dot Shawcroft at arm dot com>, Richard Earnshaw <Richard dot Earnshaw at arm dot com>, Jim Wilson <jim dot wilson at linaro dot org>
- Date: Wed, 15 Apr 2015 14:35:18 +0300
- Subject: Re: [AArch64][PR65375] Fix RTX cost for vector SET
- Authentication-results: sourceware.org; auth=none
- References: <55066BCC dot 4010900 at linaro dot org> <5506AA24 dot 3050108 at arm dot com> <5506CD7A dot 7030109 at linaro dot org> <5506D77B dot 5060909 at linaro dot org> <55070972 dot 3000800 at arm dot com> <5507813E dot 3060106 at linaro dot org> <5513B390 dot 2030201 at linaro dot org> <552D8FF7 dot 5000105 at linaro dot org> <20150415092509 dot GA20852 at arm dot com> <552E4150 dot 3020403 at linaro dot org> <20150415111854 dot GB22143 at arm dot com>
> On Apr 15, 2015, at 2:18 PM, James Greenhalgh <james.greenhalgh@arm.com> 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.
While I am for cleaning up messy code, I want to avoid Kugan's patch being held hostage until all the proper refactorings and cleanups are done. If we consider the patch on its own merits: Is it a worthwhile improvement? -- [Probably, "yes".] Does it make current spaghetti code significantly more difficult to understand? -- [Probably, "no", if we update the current comments.]
Let's discuss the effort of cleaning RTX costs as a separate task. It can be either a joint effort for ARM and Linaro, or one of us can tackle it.
Thank you,
--
Maxim Kuvyrkov
www.linaro.org