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?
We don't have any insns that match this, since all msp430 instructions are
either single or double operand.
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.
Thanks,
Jozef
>
>
> I've got no conceptual problem with the patch, I just want to make sure
> you've thought about those issues.
>
> jeff
>
>
>
