Re: [avr-gcc-list] split movhi to 2 movqi

[Georg-Johann Lay]

Ilya Lesokhin schrieb:
> Hi,
>
> I would like to split a movhi instruction from an immediate to a const
> address to 2 movqi instructions.
> (I'm using gcc 4.5.3 but i dont think it matters in this case)

The current development version is 4.7.0.

It won't help to provida a patch against an old version. So you need an 
up-to-date version anyway. This includes also the testsuite. Patches 
must pass the testsuite without regressions.

> My motivation is the following:
> Code in the form:
> OCR2RA = 1;
>
> compiles to:
>
> ldi r24, 0x01 ; 1
> ldi r25, 0x00 ; 0
> sts 0x00F7, r25
> sts 0x00F6, r24
>
> instead of
>
> ldi r24, 0x01 ; 1
> ldi r25, 0x00 ; 0
> sts 0x00F7, r25
> sts 0x00F6, __zero_reg__

What's your intention? Is it an optimization issue?

> I'm pretty sure its caused by the following code:
> (define_expand "movhi"
> [(set (match_operand:HI 0 "nonimmediate_operand" "")
> (match_operand:HI 1 "general_operand" ""))]
> ""
> "
> {
>  /* One of the ops has to be in a register. */
> if (!register_operand(operand0, HImode)
> && !(register_operand(operand1, HImode) || const0_rtx == operands[1]))
> {
> operands[1] = copy_to_mode_reg(HImode, operand1);
> }
> }")
>
> i would like to fix it but i know very little about gcc internals.
>
>
>
> i tried to do something like:
>
> (define_expand "movhi"
> [(set (match_operand:HI 0 "nonimmediate_operand" "")
> (match_operand:HI 1 "general_operand" ""))]
> ""
> "
> {
> rtx base = XEXP (operands[0], 0);
> if (GET_CODE (operands[1]) == CONST_INT
> && CONSTANT_ADDRESS_P (base))
> {
> short value = INTVAL (operands[1]);
> short address = INTVAL (base);

base is a constant, but not necessarily a CONST_INT, it can also be a 
SYMBOL_REF or a CONST. If you add support for storing zero-extended 
values, I see no reason why to excluse some flavours of address.

> emit_move_insn(gen_rtx_MEM(QImode,address+1), GEN_INT(value >> 8));
> emit_move_insn(gen_rtx_MEM(QImode,address), GEN_INT(value & 0xff));

address+1 is wrong, because address in general is not a CONST_INT
Maybe plus_constant is what you looking for.

Moreover, there is no point in expanding some stuff if there is no insn 
that will handle it.

> DONE;
> }
> else /* One of the ops has to be in a register. */
> if (!register_operand(operand0, HImode)
> && !(register_operand(operand1, HImode) || const0_rtx == operands[1]))
> {
> operands[1] = copy_to_mode_reg(HImode, operand1);
> }
> }")
>
> but then i get Segmentation fault when trying to compile code.

You move expansion cannot work because you need at least a QI register 
(except in the case when you like to store 0, which is already handled 
in the implementation.

Presumably, you want code like this:

(set (reg:QI n)
     (const_int m))
(set (mem:HI (const_addr:P))
     (zero_extend:HI (reg:QI n)))

You can do it in expand, but note that expand is called at different 
stages of compilation. You need an intermediate register, so you must be 
sure you can actually get one, i.e. can_create_pseudo_p is true.

As an alternative, you can hook in after insn combine.
combine will synthesize such insns, so you can supply a combiner pattern 
that matches and split it prior to register allocation by means of a 
split pattern.

Yet an other alternative is to extend zero_extendqihi to accept memory 
operand in dest. Note that there is different handlich for volatiles.

> Can someone please point me to the right direction?
>
> Thank,
>
> Ilya.