> > - if (GET_MODE_BITSIZE (GET_MODE (op0)) < GET_MODE_BITSIZE > > (GET_MODE (op1))) > > + /* Don't try to sign-extend SImode to PSImode, for example. */ > > + if (GET_MODE_BITSIZE (GET_MODE (op0)) <= GET_MODE_BITSIZE > > (GET_MODE (op1))) > > Can't you do just nothing if > GET_MODE_BITSIZE (GET_MODE (op0)) == GET_MODE_BITSIZE (GET_MODE (op1))?
Truncating SImode to PSImode is a real operation on the MSP430, so I can't just "do nothing". > For UNSPECs and UNSPECVs you should use define_c_enum: > > (define_c_enum "unspec" [ Didn't know you could do that, but OK. > > +(define_insn "popm" > > + [(unspec_volatile [(match_operand 0 "register_operand" "r") > > + (match_operand 1 "immediate_operand" "i") > > + (match_operand 2 "immediate_operand" "i")] UNS_POPM)] > > + "" > > + "POPM%B0\t%2, r%I1" > > + ) > > I don't understand what the problem is here. If the register is > modified by the post-reload passes, apparently there is a side-effect > in this insn that isn't modeled properly, and the pattern hacks around > the real problem. Is it an auto-inc-dec thing? Is it a SET? Something > CLOBBERED? This is only used by the epilogue, it pops a *range* of registers in a single operation. I.e. it's the same as "pop r4; pop r5; pop r6; ..." Are you saying I need to attach the chain of regnotes to tell the compiler about all those pops? I did the pushm one because the dwarf2 stack stuff needed to know, but it doesn't need to know about this one. > > +(define_expand "movsi" > > + [(set (match_operand:SI 0 "nonimmediate_operand" "=rm") > > + (match_operand:SI 1 "general_operand" "rmi"))] > > + "" > > + "" > > + ) > > What is the define_expand for? If you omit this, the SImode moves are split into HImode moves before RTL expansion, and we lose some opportunities for optimization. Or did you mean "why didn't you just name the insn_and_split movsi"? If so, I don't recall why I did them separately... > > +; NOTE - we are playing a dangerous game with GCC here. We have these two > > +; add patterns and the splitter that follows because our tests have shown > > +; that this results in a significant reduction in code size - because GCC > > is > > +; able to discard any unused part of the addition. We have to annotate the > > +; patterns with the set and use of the carry flag because otherwise GCC > > will > > +; discard parts of the addition when they are actually needed. But we have > > +; not annotated all the other patterns that set the CARRY flag as doing so > > +; results in an overall increase in code size[1]. Instead we just *hope* > > +; that GCC will not move a carry-setting instruction in between the first > > +; and second adds. > > Scary stuff... :-) Agreed, but testing says it works... > > Index: gcc/config/msp430/t-msp430 > > =================================================================== > > --- gcc/config/msp430/t-msp430 (revision 0) > > +++ gcc/config/msp430/t-msp430 (revision 0) > > @@ -0,0 +1,43 @@ > > +# Makefile fragment for building GCC for the Renesas RX target. > > +# Copyright (C) 2008, 2009, 2010, 2011 Free Software Foundation, Inc. > > Copyright 2012-2013, and not for the RX target. Heh. Guess which port I used as a template? ;-) > > +#undef TARGET_HANDLE_OPTION > > +#define TARGET_HANDLE_OPTION msp430_handle_option > > Remove the no-op hook and keep the default? I can, but what happens if the default changes? > > +/* We use this to wrap all emitted insns in the prologue. */ > > +static rtx > > +F (rtx x) > > Perhaps a more descriptive function name? ;-) Only if print-rtl does something more than "/f" on the insn ;-) I've used this in other ports, it's used in expressions so brevity is important to keep expressions short.