On 10/14/19 7:30 AM, Jozef Lawrynowicz wrote:
> On Sun, 13 Oct 2019 10:22:31 -0600
> Jeff Law <l...@redhat.com> wrote:
>
>> On 10/7/19 2:28 PM, Jozef Lawrynowicz wrote:
>>> MSP430 supports post increment addressing for the source operand only. The
>>> attached patch enables post increment addressing for MSP430 in GCC.
>>>
>>> Regtested on trunk for the small and large memory models.
>>>
>>> Ok for trunk?
>>>
>>>
>>> 0001-MSP430-Implement-post-increment-addressing.patch
>>>
>>> From d75e48ba434d7bab141c09d442793b2cb26afa69 Mon Sep 17 00:00:00 2001
>>> From: Jozef Lawrynowicz <joze...@mittosystems.com>
>>> Date: Mon, 16 Sep 2019 16:34:51 +0100
>>> Subject: [PATCH] MSP430: Implement post increment addressing
>>>
>>> gcc/ChangeLog:
>>>
>>> 2019-10-07 Jozef Lawrynowicz <joze...@mittosystems.com>
>>>
>>> * config/msp430/constraints.md: Allow post_inc operand for "Ya"
>>> constraint.
>>> * config/msp430/msp430.c (msp430_legitimate_address_p): Handle
>>> POST_INC.
>>> (msp430_subreg): Likewise.
>>> (msp430_split_addsi): Likewise.
>>> (msp430_print_operand_addr): Likewise.
>>> * config/msp430/msp430.h (HAVE_POST_INCREMENT): Define.
>>> (USE_STORE_POST_INCREMENT): Define.
>>> * config/msp430/msp430.md: Use the msp430_general_dst_operand or
>>> msp430_general_dst_nonv_operand predicates for the lvalues insns.
>>> * config/msp430/predicates.md (msp430_nonpostinc_operand): New.
>>> (msp430_general_dst_operand): New.
>>> (msp430_general_dst_nonv_operand): New.
>>> (msp430_nonsubreg_operand): Remove.
>>> (msp430_nonsubreg_dst_operand): New.
>>> (msp430_nonsubreg_or_imm_operand): Allow reg or mem operands in place
>>> of defunct msp430_nonsubreg_operand.
>>> (msp430_nonsubregnonpostinc_or_imm_operand): New.
>> So a high level question. The
>> USE_STORE_{PRE,POST}_{INCREMENT,DECREMENT} are primarily used within the
>> ivopts code to tune the addressing modes generated in there.
>>
>> To the best of my knowledge, they do not totally prevent using those
>> addressing modes elsewhere (auto-inc-dec.c) which instead rely strictly
>> on the HAVE_ macros and recognizing the result against the MD file.
>
> Additionally I don't think inc/dec RTXs will ever actually be generated unless
> they are handled in TARGET_LEGITIMATE_ADDRESS_P.
>
>>
>> So will these changes handle auto-increment addressing modes in
>> destination operands if they are generated by auto-inc-dec.c? Or have
>> you fixed all the predicates so that auto-inc-dec.c won't create them in
>> the first place?
>
> Yes, the new msp430_*_dst_operand predicates I've added will disallow a
> (mem (post_inc)).
>
>>
>>
>>
>> Based on a comment in msp430_split_addsi you have to handle stuff like
>>
>>> + (set (reg:SI)
>>> + (plus:SI (reg:SI)
>>> + (mem:SI (post_inc:PSI (reg:PSI))))).
>>
>> Do you need to check for and do anything special if the destination
>> operand is the same as the post-inc operand. Note RTX equality test is
>> probably not sufficient since you've got differing modes. Note this may
>> be affected by the prior question...
>
> This shouldn't ever happen since every lvalue operand which could be a mem
> has a
> dst_operand predicate. msp430_legitimate_address_p only allows post_inc in the
> format (post_inc (reg)) so we'll only ever get (mem (post_inc (reg))) RTXs.
>>
>> Are there any places where you could potentially have two source memory
>> operands? If so, do you need additional checks in those patterns?
>
> Are you referring to places where we have two source memory operands, neither
> of which are equal to the dest?
Yes.
>
> We don't have any insns that match this, since all msp430 instructions are
> either single or double operand.
Perfect.
>
> For expand patterns that could have two source memory operands,
> msp430_expand_helper will always move the operands into registers before
> calling a helper function.
>
> And the splitter for addsi is the only other place where we could have two
> source memory operands. This is where I put in two predicates which disallow
> post_inc, so there is only one possible position we can have it. IIRC, this
> case
> was actually exposed by running the testsuite :)
>
> Also, the assembler will emit the following error message if we ever do
> generate a post_inc for the dest operand:
>> t.s: Assembler messages:
>> t.s:2: Error: this addressing mode is not applicable for destination operand
>
> So I'm quite satisfied that can't/doesn't happen.
Perfect. THanks for walking me through things. OK for the trunk.
jeff
