Re: no_new_pseudos

[Alexandre Oliva]

On Jul  6, 2007, Ian Lance Taylor <[EMAIL PROTECTED]> wrote:

> Richard Sandiford <[EMAIL PROTECTED]> writes:
>> That's why it seems so odd to me to want to get rid of the port uses
>> and not replace it with something directly equivalent.  I just don't
>> see how it qualifies as a clean-up.  I think tying the ports even
>> more to reload-specific conditions, even when we already have a more
>> abstract concept, is the wrong way to go.

> At the risk of disturbing the bikeshed painting, what do you think of
> this patch?

It's better, because it retains an easy-to-search-for interface, but
it still doesn't match the abstraction expected by back-end uses.  Let
me try to explain why it is important to retain this interface.

First, a trivial nit: regalloc_started_p should become true when
local(-regalloc) started, and I assume you meant it to become true
only when reload started.  This might seem like an irrelevant point,
but I'll show below why it is more relevant than it seems.

regalloc_started_p is still at a lower level of abstraction than
no_new_pseudos.  Just because no_new_pseudos can be currently
implemented in terms of regalloc_started_p, it's not obvious that it
will remain so.

Consider changes in reload that enable it to cope with the creation of
new pseudos up to a certain point.  Would we then have to change all
occurrences of regalloc_started_p() that mean no_new_pseudos to
(regalloc_started_p() && !new_pseudos_still_ok_p ()).

Consider that we drop reload entirely and come up with a global
allocator that can cope with new pseudos up to a certain point, but
not all the way to the end.  Do we then change regalloc_started_p()
such that it becomes true only in the middle of this global allocator?
Or audit all uses of regalloc_started_p() so as to verify wether it
means no_new_pseudos or actually regalloc_started_p?

Consider that local and or global are changed in such a way that they
start using expanders that currently use no_new_pseudos.  Well, surely
expanders that create new pseudos after local or global get poorer
register allocation than those that run before register allocation
starts.  Someone looking at the backends might think
regalloc_started_p() is there for performance reasons, rather than for
correctness reasons, and make incorrect decisions based on that.  At
some point, it may be difficult to tell whether certain uses were
mis-uses based on an incorrect understanding of what
regalloc_started_p really stood for, because it could arguably mean
both.  Some might even be tempted to change its definition such that
it is "more correct", and then ports would break that depended on the
semantics of no_new_pseudos to work.

Arguably, we should have another abstraction to enable ports to tell
not only whether it's *possible* to create new pseudos, but whether
it's *efficient* to do so.  This pair of abstractions could then be
implemented in terms of a similar enumeration to the one I suggested
before, but adding yet another value:

enum { BEFORE_REGALLOC = -2,
   DURING_REGALLOC = -1,
   DURING_RELOAD = 0,
   AFTER_RELOAD = 1
 } regalloc_state;

#define reload_in_progress (regalloc_state == DURING_RELOAD)
#define reload_completed (regalloc_state > DURING_RELOAD)
#define no_new_pseudos (regalloc_state >= DURING_RELOAD)
#define inefficient_new_pseudos (regalloc_state > BEFORE_REGALLOC)

And then, had we introduced this abstraction you proposed, we might
end up with something like:

#define regalloc_started_p() (regalloc_state >= DURING_RELOAD)
#define real_regalloc_started_p() (regalloc_state >= DURING_REGALLOC)

See why imprecise abstractions are a problem, and why lowering
abstractions just because it's possible ATM, without any performance
or maintainability gains to justify them, is a losing proposition in
the long run?

-- 
Alexandre Oliva http://www.lsd.ic.unicamp.br/~oliva/
FSF Latin America Board Member http://www.fsfla.org/
Red Hat Compiler Engineer   [EMAIL PROTECTED], gcc.gnu.org}
Free Software Evangelist  [EMAIL PROTECTED], gnu.org}

Re: Q: middle-end problem when variadic builtins promote float to double

[Richard Guenther]

On 7/8/07, Kaveh R. GHAZI <[EMAIL PROTECTED]> wrote:

On Sat, 7 Jul 2007, Joseph S. Myers wrote:

> No, that's something else entirely (a "float" old-style parameter
> declaration corresponds to a "double" argument in a prototype).  It's
> convert_arguments that handles converting to prototype types and default
> argument promotions for arguments not covered by a prototype (including
> those in the ... of a variadic function).
>
>   else if (TREE_CODE (TREE_TYPE (val)) == REAL_TYPE
>&& (TYPE_PRECISION (TREE_TYPE (val))
>< TYPE_PRECISION (double_type_node))
>&& !DECIMAL_FLOAT_MODE_P (TYPE_MODE (TREE_TYPE (val
> /* Convert `float' to `double'.  */
> argarray[parmnum] = convert (double_type_node, val);


Ah perfect, thanks.  I'm thinking something like what's below.  I'll move
it over to gcc-patches and add a ChangeLog if it passes testing.

Thanks for your help!

--Kaveh


diff -rup orig/egcc-SVN20070706/gcc/c-typeck.c egcc-SVN20070706/gcc/c-typeck.c
--- orig/egcc-SVN20070706/gcc/c-typeck.c2007-06-30 23:02:59.0 
-0400
+++ egcc-SVN20070706/gcc/c-typeck.c 2007-07-07 21:26:33.982197838 -0400
@@ -2394,6 +2394,8 @@ convert_arguments (int nargs, tree *arga
 {
   tree typetail, valtail;
   int parmnum;
+  const bool type_generic =
+!!lookup_attribute ("type generic", TYPE_ATTRIBUTES(TREE_TYPE (fundecl)));
   tree selector;

   /* Change pointer to function to the function itself for
@@ -2585,8 +2587,13 @@ convert_arguments (int nargs, tree *arga
   && (TYPE_PRECISION (TREE_TYPE (val))
   < TYPE_PRECISION (double_type_node))
   && !DECIMAL_FLOAT_MODE_P (TYPE_MODE (TREE_TYPE (val
-   /* Convert `float' to `double'.  */
-   argarray[parmnum] = convert (double_type_node, val);
+{
+ /* Convert `float' to `double'.  */
+ if (type_generic)
+   argarray[parmnum] = val;
+ else
+   argarray[parmnum] = convert (double_type_node, val);
+   }
   else if ((invalid_func_diag =
targetm.calls.invalid_arg_for_unprototyped_fn (typelist, 
fundecl, val)))
{


So type-generic is supposed to apply to scalar floating point types
only?  Btw. you need to add documentation for this function attribute.

Richard.

Re: no_new_pseudos

[Ian Lance Taylor]

Alexandre Oliva <[EMAIL PROTECTED]> writes:

> See why imprecise abstractions are a problem, and why lowering
> abstractions just because it's possible ATM, without any performance
> or maintainability gains to justify them, is a losing proposition in
> the long run?

To be blunt: no, I don't.  I see a set of hypothetical possibilities,
none of which I consider to be at all likely.

Any decision can be argued endlessly.  That's why this is a bikeshed.
Two patches have been proposed.  For a bikeshed, real code wins over
abstract discussion.

More specifically, backend routines are never called arbitrarily or
randomly.  The backends already know that only the insns which have to
check no_new_pseudos are the move expanders and the splitters.  For
example, look for calls to gen_reg_rtx in i386.md.  Most of them do
not check no_new_pseudos, and they don't have to.  If you want your
more general semantics, don't you logically have to change that too?
After all, just as the register allocator might somehow support
creating new pseudo registers in the middle of it, the register
allocator might similarly support changing the operations which were
done.  But since these aspects of the register allocator are not at
all likely to change, wouldn't it be a waste of time to prepare for
them now?

I have never liked no_new_pseudos, because it is a negative flag and
therefore confusing to use.  It is also currently carries no meaning.
The former meaning was not as crisp as you would like it to be: it was
there to remind people to call the appropriate functions to resize the
register arrays.  Only it didn't express that very well, so for
complete correctness you had to actually check both (no_new_pseudos &&
(reload_in_progress || reload_completed)).  Only nobody bothered to do
that, because passes generally knew whether they were run before or
after register allocation.  Note that because of this the backend
routines treated the variable differently from the frontend routines.
For these reasons, since the underlying meaning of no_new_pseudos is
no longer necessary, I am seizing the opportunity to eliminate it.

Ian

Re: Q: middle-end problem when variadic builtins promote float to double

[Kaveh R. GHAZI]

On Sun, 8 Jul 2007, Richard Guenther wrote:

> So type-generic is supposed to apply to scalar floating point types
> only?

So far, yes.

I don't see anything that requires or prohibits changing that for the
initial implementation.  If later a GCC developer wants to change it to
not promote e.g. char, that could be a refinement.  I don't think any of
the type-generic builtins allows non-floats anyway, so it would have no
effect to have it or not at this point.


>  Btw. you need to add documentation for this function attribute.

The internal "no vops" builtin isn't documented AFAICT, I was just
following that example.  We don't want to confuse users into thinking they
can use it IMHO.  What do you suggest?

--Kaveh
--
Kaveh R. Ghazi  [EMAIL PROTECTED]

Re: Q: middle-end problem when variadic builtins promote float to double

[Richard Guenther]

On 7/8/07, Kaveh R. GHAZI <[EMAIL PROTECTED]> wrote:

On Sun, 8 Jul 2007, Richard Guenther wrote:

> So type-generic is supposed to apply to scalar floating point types
> only?

So far, yes.

I don't see anything that requires or prohibits changing that for the
initial implementation.  If later a GCC developer wants to change it to
not promote e.g. char, that could be a refinement.  I don't think any of
the type-generic builtins allows non-floats anyway, so it would have no
effect to have it or not at this point.


Ok.



>  Btw. you need to add documentation for this function attribute.

The internal "no vops" builtin isn't documented AFAICT, I was just
following that example.  We don't want to confuse users into thinking they
can use it IMHO.  What do you suggest?


Ah, I didn't notice that it had a space in its name.  I guess not documenting
it is ok then.

Richard.

Re: no_new_pseudos

[Kenneth Zadeck]

Ian Lance Taylor wrote:
> Alexandre Oliva <[EMAIL PROTECTED]> writes:
>
>   
>> See why imprecise abstractions are a problem, and why lowering
>> abstractions just because it's possible ATM, without any performance
>> or maintainability gains to justify them, is a losing proposition in
>> the long run?
>> 
>
> To be blunt: no, I don't.  I see a set of hypothetical possibilities,
> none of which I consider to be at all likely.
>
>   

To be even more blunt, I never viewed no_new_pseudos as a useful abstraction
It was a gate that protected a set of badly designed concrete
datastructures.
Those datastructures are gone, and since I (perhaps even we) never
viewed it as being an abstraction, I (we) are confused at the discussion
of what to replace the "abstraction" with.

I am a middle-end person with only cursory knowledge of the backends. 
>From the middle end's point of view, casting the pseudos in terms of
reload/register allocation is in fact exactly the right abstraction. 
Before register allocation, you have an infinite number of registers and
after register allocation, you only have what the machine provides. 
No_new_psuedos was there to put an extra layer there because of a set of
unfortunate datastructure decisions, but the proper interface for the
backends is that they can behave in a manner where they have in infinite
number or registers before register allocation and only the fixed
registers after register allocation.

I would very much like ian to put in his patch and i will redo my patch
for the middle end and lets move on.

kenny

> Any decision can be argued endlessly.  That's why this is a bikeshed.
> Two patches have been proposed.  For a bikeshed, real code wins over
> abstract discussion.
>
> More specifically, backend routines are never called arbitrarily or
> randomly.  The backends already know that only the insns which have to
> check no_new_pseudos are the move expanders and the splitters.  For
> example, look for calls to gen_reg_rtx in i386.md.  Most of them do
> not check no_new_pseudos, and they don't have to.  If you want your
> more general semantics, don't you logically have to change that too?
> After all, just as the register allocator might somehow support
> creating new pseudo registers in the middle of it, the register
> allocator might similarly support changing the operations which were
> done.  But since these aspects of the register allocator are not at
> all likely to change, wouldn't it be a waste of time to prepare for
> them now?
>
> I have never liked no_new_pseudos, because it is a negative flag and
> therefore confusing to use.  It is also currently carries no meaning.
> The former meaning was not as crisp as you would like it to be: it was
> there to remind people to call the appropriate functions to resize the
> register arrays.  Only it didn't express that very well, so for
> complete correctness you had to actually check both (no_new_pseudos &&
> (reload_in_progress || reload_completed)).  Only nobody bothered to do
> that, because passes generally knew whether they were run before or
> after register allocation.  Note that because of this the backend
> routines treated the variable differently from the frontend routines.
> For these reasons, since the underlying meaning of no_new_pseudos is
> no longer necessary, I am seizing the opportunity to eliminate it.
>
> Ian
>   

Re: RFC: GIMPLE tuples. Design and implementation proposal

[Diego Novillo]

On 7/7/07 7:04 AM, [EMAIL PROTECTED] wrote:

> If you ask nicely they would let you use CIL's code in GCC ;) .

Any specific reasons why we should?  Better memory savings?  Faster
processing?  It's not clear from your message what the advantages would
be (ignoring the fact that their implementation language is completely
different).

Thanks.

Re: Ongoing bootstrap failures on ppc64 since 2007-07-02

[Steve Kargl]

Dorit sent me a copy of the libgfortran/config.log on
her failing system.  In looking through the log, I've
found (note, I've paths to ABC, XYZ for shortness):


configure:11335: checking if ABC/./gcc/gfortran -BABC/./gcc/ -BXYZ/bin/
  -BXYZ/lib/ -isystem XYZ/include -isystem XYZ/sys-include supports -c
  -o file.o
configure:11382: result: no
configure:11412: checking whether the ABC/./gcc/gfortran -BABC/./gcc/
  -BXYZ/bin/ -BXYZ/lib/ -isystem XYZ/include -isystem XYZ/sys-include
  linker (ABC/./gcc/collect-ld -m elf64ppc) supports shared libraries
configure:12487: result: yes
configure:12624: checking dynamic linker characteristics
configure:13000: ABC/./gcc/gfortran -BABC/./gcc/ -BXYZ/bin/ -BXYZ/lib/ -isystem 
XYZ/include -isystem XYZ/sys-include -o conftest   -Wl,-rpath -Wl,/foo  
conftest.f  >&5
conftest.f: In function 'MAIN__':
conftest.f:1: internal compiler error: tree check: expected parm_decl,
  have function_decl in set_tree_decl_type_code, at fortran/trans-decl.c:2869
Please submit a full bug report,
with preprocessed source if appropriate.
See http://gcc.gnu.org/bugs.html> for instructions.
configure:13006: $? = 1
configure: failed program was:
|   program main
| 
|   end
configure:13243: result: GNU/Linux ld.so
configure:13287: checking how to hardcode library paths into programs
configure:13312: result: immediate

configure:13356: checking whether the GNU Fortran compiler is working
configure:13370: ABC/./gcc/gfortran -BABC/./gcc/ -BXYZ/bin/ -BXYZ/lib/
  -isystem XYZ/include -isystem XYZ/sys-include -c   conftest.f >&5
conftest.f: In function 'MAIN__':
conftest.f:2: internal compiler error: tree check: expected parm_decl,
have function_decl in set_tree_decl_type_code, at fortran/trans-decl.c:2869
Please submit a full bug report,
with preprocessed source if appropriate.
See http://gcc.gnu.org/bugs.html> for instructions.
configure:13376: $? = 1
configure: failed program was:
| 
|   program foo
|   real, parameter :: bar = sin (12.34 / 2.5)
|   end program foo
configure:13397: result: no
configure:13399: error: GNU Fortran is not working; please report
   a bug in http://gcc.gnu.org/bugzilla, attaching ABC/XYZ/libgfortran/\
config.log

Neither of these codes should result in a code path to 
set_tree_decl_type_code() in fortran/trans-decl.c.   The only
use of this function is in this section of code:

  if (sym->attr.dummy == 1)
{
  /* The sym->backend_decl can be NULL if this is one of the
intrinsic types, such as the symbol of type c_ptr for the
c_f_pointer function, so don't set up the tree code for it.  */
  if (sym->attr.value == 1 && sym->backend_decl != NULL)
   set_tree_decl_type_code (sym);
}

The newly built gfortran must be stomping on memory.  I've found that
attached patch allows gfortran to still function.  Could someone who
sees this problem try bootstrapping gfortran with the patch?

-- 
Steve
Index: trans-decl.c
===
--- trans-decl.c	(revision 126447)
+++ trans-decl.c	(working copy)
@@ -2850,7 +2850,7 @@
approach the C compiler takes (or it appears to be this way).  When
the middle-end is given the typed node rather than the POINTER_TYPE
node, it knows to pass the value.  */
-
+#if 0
 static void
 set_tree_decl_type_code (gfc_symbol *sym)
 {
@@ -2873,6 +2873,7 @@
 
return;
 }
+#endif
 
 
 /* Drill down through expressions for the array specification bounds and
@@ -3024,14 +3025,14 @@
 	}
 }
 
-  if (sym->attr.dummy == 1)
-{
-  /* The sym->backend_decl can be NULL if this is one of the
-	 intrinsic types, such as the symbol of type c_ptr for the
-	 c_f_pointer function, so don't set up the tree code for it.  */
-  if (sym->attr.value == 1 && sym->backend_decl != NULL)
-	set_tree_decl_type_code (sym);
-}
+#if 0
+  /* The sym->backend_decl can be NULL if this is one of the intrinsic types,
+ such as the symbol of type c_ptr for the c_f_pointer function, so don't
+ set up the tree code for it.  */
+  if (sym->attr.flavor != FL_PROCEDURE && sym->attr.dummy == 1
+  && sym->attr.value == 1 && sym->backend_decl != NULL)
+set_tree_decl_type_code (sym);
+#endif
 
   /* Make sure we convert the types of the derived types from iso_c_binding
  into (void *).  */

Re: no_new_pseudos

[Alexandre Oliva]

On Jul  8, 2007, Ian Lance Taylor <[EMAIL PROTECTED]> wrote:

> But since these aspects of the register allocator are not at all
> likely to change, wouldn't it be a waste of time to prepare for them
> now?

Yup.  But from that to concluding that we should remove the clear
abstraction that enables someone to prepare for them right now, that
is useful, mnemonic, clear in meaning, and currently functional.

Replacing that with some variable that denotes some internal state in
the middle end and requiring the back end to use it is exposing the
guts of the middle end to the back end.  That's breaking abstraction
layers.  That's bad software engineering in general.

> I have never liked no_new_pseudos, because it is a negative flag and
> therefore confusing to use.

Oh, if that's all, we can address that.

  s,no_new_pseudos,!new_pseudos_acceptable_p (),g

would address it, without exposing the guts of the middle end to the
back end.

> The former meaning was not as crisp as you would like it to be: it was
> there to remind people to call the appropriate functions to resize the
> register arrays.

That's just another implementation detail of the middle end that back
ends shouldn't have to be concerned with.  Their concern is whether
they can introduce new pseudos or not.  no_new_pseudos provided that
meaning precisely.

> Only it didn't express that very well, so for
> complete correctness you had to actually check both (no_new_pseudos &&
> (reload_in_progress || reload_completed)).

This is news to me.  Why would that be?  Isn't no_new_pseudos always
set when reload_in_progress || reload_completed holds?  If not, then
the proposed change may indeed be harmful to backends.  It seems to be
an indication that the current abstraction is not suitable for all
users.  Replacing it for another abstraction that is not suitable for
all users doesn't seem like a solution, since it merely moves the
problem elsewhere.

> For these reasons, since the underlying meaning of no_new_pseudos is
> no longer necessary,

As in, we can now create new pseudos at any time throughout the
compilation and expect them to be handled correctly?  I don't think
so.

-- 
Alexandre Oliva http://www.lsd.ic.unicamp.br/~oliva/
FSF Latin America Board Member http://www.fsfla.org/
Red Hat Compiler Engineer   [EMAIL PROTECTED], gcc.gnu.org}
Free Software Evangelist  [EMAIL PROTECTED], gnu.org}

Re: no_new_pseudos

[Alexandre Oliva]

On Jul  8, 2007, Kenneth Zadeck <[EMAIL PROTECTED]> wrote:

> To be even more blunt, I never viewed no_new_pseudos as a useful abstraction
> It was a gate that protected a set of badly designed concrete
> datastructures.

I can appreciate that this is a valid point of view for the
implementation of the data structures.  But APIs are not designed only
for the implementation of the data structures.  APIs ought to be
geared toward the *users* of the implementation.  In this case, the
backend is one of the users, and no_new_pseudos provided it with a
meaningful way to test whether creating new pseudos was still
acceptable.  Replacing that with concepts that reflect internal
implementation details, rather than something that is meaningful for
the users of the API, is offering a worse API to the users.

-- 
Alexandre Oliva http://www.lsd.ic.unicamp.br/~oliva/
FSF Latin America Board Member http://www.fsfla.org/
Red Hat Compiler Engineer   [EMAIL PROTECTED], gcc.gnu.org}
Free Software Evangelist  [EMAIL PROTECTED], gnu.org}

Re: Ongoing bootstrap failures on ppc64 since 2007-07-02

[Revital1 Eres]

> The newly built gfortran must be stomping on memory.  I've found that
> attached patch allows gfortran to still function.  Could someone who
> sees this problem try bootstrapping gfortran with the patch?

I will try it.

Revital

Re: no_new_pseudos

[Ian Lance Taylor]

Alexandre Oliva <[EMAIL PROTECTED]> writes:

> On Jul  8, 2007, Ian Lance Taylor <[EMAIL PROTECTED]> wrote:
> 
> > But since these aspects of the register allocator are not at all
> > likely to change, wouldn't it be a waste of time to prepare for them
> > now?
> 
> Yup.  But from that to concluding that we should remove the clear
> abstraction that enables someone to prepare for them right now, that
> is useful, mnemonic, clear in meaning, and currently functional.
> 
> Replacing that with some variable that denotes some internal state in
> the middle end and requiring the back end to use it is exposing the
> guts of the middle end to the back end.  That's breaking abstraction
> layers.  That's bad software engineering in general.

Note that that is exactly what was happening before.  no_new_pseudos
denoted internal state in the middle end.  You are repurposing it from
something bad to something good.

But I believe that repurposing is bad because the backends do not and
should not use no_new_pseudos consistently.  Consistent usage would
require them to check it far more often than they currently do.  That
would be counterproductive and even impossible.  Instead, the backends
only check it for the specific routines which may be called during
register allocation.

Since those are the only backend routines which must check
no_new_pseudos, and those are the only backend routines which do check
no_new_pseudos, I think that trying to reclaim no_new_pseudos as a
generic concept separate from register allocation is inappropriate.


> > I have never liked no_new_pseudos, because it is a negative flag and
> > therefore confusing to use.
> 
> Oh, if that's all, we can address that.
> 
>   s,no_new_pseudos,!new_pseudos_acceptable_p (),g
> 
> would address it, without exposing the guts of the middle end to the
> back end.

I will do that if there is a general consensus that this is a good
idea.  I personally believe that you are striving for a distinction
which should not exist.  Since we have no reason to believe that the
backend should not create new pseudo-registers before register
allocation, and since we have no reason to believe that after register
allocation starts it will be possible for the backend to create new
pseudo-registers, I believe that you are introducing a layer of
abstraction which does not clarify.

Perhaps a root of the disagreement is that I believe that unnecessary
abstraction is actually harmful.  We are not writing a general purpose
library or API here, for either the middle-end or the backend.  We are
writing a tightly coupled single program.


> > Only it didn't express that very well, so for
> > complete correctness you had to actually check both (no_new_pseudos &&
> > (reload_in_progress || reload_completed)).
> 
> This is news to me.  Why would that be?  Isn't no_new_pseudos always
> set when reload_in_progress || reload_completed holds?  If not, then
> the proposed change may indeed be harmful to backends.  It seems to be
> an indication that the current abstraction is not suitable for all
> users.  Replacing it for another abstraction that is not suitable for
> all users doesn't seem like a solution, since it merely moves the
> problem elsewhere.

I was thinking of the middle-end here, not the backend, but I'll
withdraw the comment.  The situation before DF was confusing, but it
was not difficult to handle correctly.


> > For these reasons, since the underlying meaning of no_new_pseudos is
> > no longer necessary,
> 
> As in, we can now create new pseudos at any time throughout the
> compilation and expect them to be handled correctly?  I don't think
> so.

Since that would be obviously nonsensical, it must not be what I
meant.  I must have meant what I said: "it was there to remind people
to call the appropriate functions to resize the register arrays."  I
believe that you are restating the underlying meaning of
no_new_pseudos from what it originally was.

Ian

Re: Ongoing bootstrap failures on ppc64 since 2007-07-02

[Revital1 Eres]

> The newly built gfortran must be stomping on memory.  I've found that
> attached patch allows gfortran to still function.  Could someone who
> sees this problem try bootstrapping gfortran with the patch?

gfortran bootstrapped OK with this patch on ppc64 r126353.

Thanks,
Revital