This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] Enhance reload_cse_move2add


On 07/13/2010 12:50 PM, Jie Zhang wrote:
On 07/13/2010 02:38 AM, H.J. Lu wrote:
On Mon, Jul 12, 2010 at 11:36 AM, H.J. Lu<hjl.tools@gmail.com> wrote:
On Mon, Jul 12, 2010 at 10:41 AM, Jie Zhang<jie@codesourcery.com> wrote:
On 07/13/2010 12:37 AM, Jeff Law wrote:

On 06/30/10 01:45, Jie Zhang wrote:

Currently reload_cse_move2add can transform


(set (REGX) (CONST_INT A))
...
(set (REGX) (CONST_INT B))

to

(set (REGX) (CONST_INT A))
...
(set (REGX) (plus (REGX) (CONST_INT B-A)))

This patch enhances it to be able to transform

(set (REGX) (CONST (PLUS (SYMBOL_REF) (CONST_INT A))))
...
(set (REGY) (CONST (PLUS (SYMBOL_REF) (CONST_INT B))))

to

(set (REGX) (CONST (PLUS (SYMBOL_REF) (CONST_INT A))))
...
(set (REGY) (CONST (PLUS (REGX) (CONST_INT B-A))))


Benchmarking using EEMBC on ARM Cortex-A8 shows performance improvement on one test:

idctrn01: 6%

No meaningful changes on other tests.

Benchmarking using SPEC2000 on AMD Athlon64 X2 3800+ shows 0.4%
regression on CINT2000 and 0.1% improvement on CFP2000.

Bootstrapped and regression tested on x86_64.

Is it OK?

Thanks for your patience, particularly when my requests for additional data. I think we're OK with the benchmarking #s, particularly looking at the amount of noise in the runs we're seeing for x86_64.

Appoved,

Jeff,

Thanks! I learned a lot during the procedure.

On 07/10/2010 08:06 AM, Bernd Schmidt wrote:
So what kinds of changes are there that would explain a 1.48% drop on
179.art? If you can verify that the code is in fact identical, and the
performance change is noise, the patch is ok.

Bernd,

I didn't realize it was a conditional approval until I asked you
privately.
Thanks!


I have committed it on trunk.



I think it breaks bootstrap on Linux/ia32:


../../src-trunk/gcc/postreload.c: In function 'reload_cse_regs':
../../src-trunk/gcc/postreload.c:1327:57: error: 'min_regno' may be
used uninitialized in this function [-Werror=uninitialized]
../../src-trunk/gcc/postreload.c:1284:7: note: 'min_regno' was
declared here
cc1: all warnings being treated as errors


I opened:


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=44921

Thanks for catching that. I did the bootstraping and regression testing
when I submitted my patch. But I didn't do them before committing.

I saw your have already committed the fix:

http://gcc.gnu.org/ml/gcc-patches/2010-07/msg01021.html

But as amylaar (sorry I don't know the real name since the bugzilla is
broken now) pointed out in the bugzilla, this is a false warning. I did
several testing of my patch with old revisions this morning. I found
r161569 with my patch bootstrap OK. r161614 with my patch fails
bootstrap. So there is some revision between them caused the false
warning and we need to find out which and if it's better to fix that
revision than adding initialization to min_regno.

It's revision 161605. I will add a followup to the corresponding thread.


-- Jie Zhang CodeSourcery


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]