Re: [Patch, AVR]: Fix PR46779

[Denis Chertykov <chertykov at gmail dot com>]

2011/7/7 Georg-Johann Lay <avr@gjlay.de>:
> Denis Chertykov wrote:
>> 2011/6/27 Georg-Johann Lay:
>>> Denis Chertykov wrote:
>
>>>> The main problem for me is that the new addressing mode produce a
>>>> worse code in many tests.
>>> You have an example source?
>>
>> In attachment.
>>
>> Denis.
>
> Hi Denis.
>
> I had a look at the sources you sent.
>
> sort.c:
> =======
>
> There is some difference because of register allocation, but the new
> code does not look awfully bad, just a bit different because of
> different register allocation that might need some more bytes.
>
> The difference is *not* because of deny fake X addressing, it's
> because of the new avr_hard_regno_mode_ok implementation to fix
> PR46779. ÂWhen I add
>
>
> Âif (GET_MODE_SIZE (mode) == 1)
> Â Â return 1;
>
> + Â Â if (SImode == mode && regno == 28)
> + Â Â Â return 0;
>
> Â return regno % 2 == 0;
>
> to that function, the difference in code disappears.
>


I have attached my version of addressing experiments.
(on rietveld http://codereview.appspot.com/4674046/)
It's against Richard's git://gcc.gnu.org/git/gcc.git rth/avr-addressing
(may be you compare my regressions with your. It's interesting.)

I still have a regression for sort.c (but I didn't analyzed the code):
   text	   data	    bss	    dec	    hex	filename
    364	      0	      0	    364	    16c	/tmp/svn.o
    446	      0	      0	    446	    1be	/tmp/addr.o

> pr.c:
> =====
>
> I get the following sizes with pr-0 the original compile and pr qith
> my patch:
>
>>avr-size pr-0.o
>  text  Âdata   bss   dec   hex filename
> Â 2824 Â Â Â24 Â Â Â 0 Â Â2848 Â Â b20 pr-0.o
>>avr-size pr.o
>  text  Âdata   bss   dec   hex filename
> Â 2564 Â Â Â24 Â Â Â 0 Â Â2588 Â Â a1c pr.o
>
> So the size actually decreased significantly. ÂAvoiding SI in
> avr_hard_regno_mode_ok like above does not change code size.

Right now I'm also has a win for pr.c with new addressing code:
   text	   data	    bss	    dec	    hex	filename
   1226	     24	      0	   1250	    4e2	/tmp/svn.o
   1176	     24	      0	   1200	    4b0	/tmp/addr.o


But, from time to time (while experimenting) regression was here.

>
> ============
>
> Note that I did *not* use the version from the git repository; I could
> not get reasonable code out of it (even after some fixes). ÂHundreds
> of testsuite crashes...

IMHO it's because of `avr_frame_pointer_required_p' and `avr_can_eliminate'.
(look at my attached patch)

>
> I used the initial patch that I posted; I attached it again for
> reference. Note that LEGITIMIZE_RELOAD_ADDRESS is still not
> implemented there.

I think you can copy it from rth/addressing.

>
> ============
>
> Did you decide about the fix for PR46779?
>
> http://gcc.gnu.org/ml/gcc-patches/2011-06/msg00810.html
>
> Is it ok to commit?

I forgot about testsuite regressions for this patch.

Denis.