Re: [ARM Patch 1/3]PR53189: optimizations of 64bit logic operation with constant

[Ramana Radhakrishnan <ramana dot radhakrishnan at linaro dot org>]

On 18 July 2012 11:12, Carrot Wei <carrot@google.com> wrote:
> On Wed, Jul 18, 2012 at 5:39 PM, Ramana Radhakrishnan
> <ramana.radhakrishnan@linaro.org> wrote:
>> On 18 July 2012 09:20, Carrot Wei <carrot@google.com> wrote:
>>> On Tue, Jul 17, 2012 at 9:47 PM, Ramana Radhakrishnan
>>> <ramana.radhakrishnan@linaro.org> wrote:
>>>> Carrot,
>>>>
>>>> Sorry about the delayed response.
>>>>
>>>> On 3 July 2012 12:28, Carrot Wei <carrot@google.com> wrote:
>>>>> On Thu, Jun 28, 2012 at 12:14 AM, Ramana Radhakrishnan
>>>>> <ramana.radhakrishnan@linaro.org> wrote:
>>>>>> On 28 May 2012 11:08, Carrot Wei <carrot@google.com> wrote:
>>>>>>> Hi
>>>>>>>
>>>>>>> This is the second part of the patches that deals with 64bit and. It directly
>>>>>>> extends the patterns anddi3, anddi3_insn and anddi3_neon to handle 64bit
>>>>>>> constant operands.
>>>>>>>
>>>>>>
>>>>>> Comments about const_di_ok_for_op still apply from my review of your add patch.
>>>>>>
>>>>>> However I don't see why and /ior / xor with constants that have either
>>>>>> the low or high parts set can't be expanded directly into ands of
>>>>>> subregs with moves of zero's or the original value especially if you
>>>>>> aren't looking at doing 64 bit operations in neon .With Neon being
>>>>>> used for 64 bit arithmetic it gets more interesting.
>>>>>>
>>>>>> Finally this should target PR target/53189.
>>>>>>
>>>>>
>>>>> Hi Ramana
>>>>>
>>>>> Thanks for the review. Following is the updated patch according to
>>>>> your comments.
>>>>
>>>> You've missed answering this part of my review :)
>>>>
>>>>>> However I don't see why and /ior / xor with constants that have either
>>>>>> the low or high parts set can't be expanded directly into ands of
>>>>>> subregs with moves of zero's or the original value especially if you
>>>>>> aren't looking at doing 64 bit operations in neon .With Neon being
>>>>>> used for 64 bit arithmetic it gets more interesting.
>>>>
>>> It has been handled by the const_ok_for_dimode_op and the output part
>>> of corresponding SI mode insn. Let's take the IOR case as an example.
>>>
>>
>> I noticed that - If I wasn't clear enough, the question was more
>> towards generating a subreg move at expand time rather than a split
>> and handling while outputting asm if you see what I mean.
>>
> I see your point now. I don't know how much better if we handle it
> earlier. Even if I generates subreg move for non-neon code at expand
> time, the latter output handling is still necessary for neon insns.

Just a quick note to let you know that I had a look this week and I'd
rather have the splitters deal with the idioms properly rather than
change the SImode patterns to deal with mov's . In theory with the
proper idiom type splitting dead loads can disappear and a number of
your tests that do and with the upper 32 bits as 0 don't really need a
load from memory and so on. I'd rather have it split before reload in
that form if possible. The neon case is something I don't yet have an
answer to but I'm gonna take a look.



regards,
ramana