On 04/11/2013 02:32 PM, David Edelsohn wrote:
On Thu, Apr 11, 2013 at 1:30 PM, Vladimir Makarov <vmaka...@redhat.com> wrote:
   Here is a patch to enable LRA for rs6000.  The patch includes code changes
in rs6000 machine-dependent parts and in LRA parts.

   I've added a new switch -mlra for rs6000 to make debugging LRA for rs6000
easier but not documented it as it will be gone at the end of stage1 (may be
with ability to use LRA for rs6000 if the results are unsatisfactory).  I
have only one question about its default value.  Currently by default LRA is
used but if you prefer opposite I can reverse it. Please just let me know
your opinion.

   The patch was successfully bootstrapped and tested on pppc64 and
x86/x86-64.

   Are rs6000 parts ok for trunk?
Vlad,

Thanks for your work on LRA and your work to convert the rs6000 port
to use the new feature.

My main question about the rs6000 parts of the patch is: what is
toc_ok_p suppose to mean?

-      if (TARGET_TOC)
+      toc_ok_p = (lra_in_progress && TARGET_CMODEL != CMODEL_SMALL
+                        && small_toc_ref (x, VOIDmode));
+      if (TARGET_TOC && ! toc_ok_p)
   return false;

This is a change to the meaning of the test with no explanation or
comment.  At least the name is confusing because it seems to be
selecting a subset of valid TOC addressing forms.  Medium and large
model TOC references before reload?

Also suffix "_ok" and "_p" seem redundant.

Maybe your intent is a boolean for "large" TOC that could be named
large_toc_p or large_toc_ok?

This is a hard question for me. Thanks for pointing this out, David. The code is 9 months old. It was introduced with lo_sum support for ppc:

http://gcc.gnu.org/ml/gcc-patches/2012-07/msg00260.html

Without this change, there are >3000 failures on GCC testsuite. If ppc target code says that the address

(lo_sum:DI (reg:DI <some pseudo>)
                (plus:DI (unspec:DI [
                            (symbol_ref:DI ("*.LANCHOR0") [flags 0x182])
                            (reg:DI 2 2)
                        ] UNSPEC_TOCREL)
                    (const_int 10 [0xa])))

is not legitimate lo_sum address, LRA tries to reload an address generating insn

<a pseudo> = (plus:DI (unspec:DI [
                         (symbol_ref:DI ("*.LANCHOR0") [flags 0x182])
                        (reg:DI 2 2)
                       ] UNSPEC_TOCREL)
              (const_int 10 [0xa]))

and that fails

I believe it was a quick hack for old address decomposition code which processed wrongly some addresses. I guess I forgot about this hack. Looking at this I don't like it now.

Probably we still need a more informative address decomposition which was rewritten by Richard Sandiford (and out of my control now). I'll investigate this more but probably another solution will be ready next week in the best case.

Reply via email to