Re: RFA: Fix other/44566

[Steven Bosscher <stevenb.gcc@gmail.com>]

On Sat, Jun 26, 2010 at 12:18 PM, Joern Rennecke <amylaar@spamcop.net> wrote:
> I have also confirmed that an i686-pc-linux-gnu X ppc-elf+spu-elf compiler
> can be built using configure options:
>  --target=ppc-elf --with-extra-target-list=spu-elf
> and building the makefile target 'all-gcc', and that the resulting cc1
> can compile a simple function for both architectures:
>
> [amylaar@laria gcc]$ cat foo.c
> int foo ()
> {
>  return 42;
> }
> [amylaar@laria gcc]$ ./cc1 -quiet -O2 foo.c
> [amylaar@laria gcc]$ cat foo.s
>        .file   "foo.c"
>        .section        ".text"
>        .align 2
>        .globl foo
>        .type   foo, @function
> foo:
>        li 3,42
>        blr
>        .size   foo, .-foo
>        .ident  "GCC: (GNU) 4.6.0 20100625 (experimental)"
> [amylaar@laria gcc]$ cat foop.c
> int foo () __attribute__ ((target_arch ("spu-elf")));
>
> int foo ()
> {
>  return 42;
> }
> [amylaar@laria gcc]$ ./cc1 -quiet -O2 foop.c
> [amylaar@laria gcc]$ cat foop.s
>        .file   "foop.c"
>        .arch   "spu-elf"
> .text
>        .align  2
>        .align  3
>        .global foo
>        .type   foo, @function
> foo:
>        il      $3,42
>        bi      $lr
>        .size   foo, .-foo
>        .ident  "GCC: (GNU) 4.6.0 20100625 (experimental)"

Also in a single file? i.e.

int foo_default_arch (void);
int foo_default_arch (void)
{
 return 42;
}

int foo_spu (void) __attribute__ ((target_arch ("spu-elf")));
int foo_spu (void)
{
 return 42;
}


> gcc/gcc:
>        * multi-target.h, README-multi-target, any-lang.c: New files.
>        * addresses.h: Include "multi-target.h" and add START_TARGET_SPECIFIC
>        and END_TARGET_SPECIFC markers.
                              ^ typo

Even though I really like the idea of supporting multi-target, this
approach looks a bit fragile to me. That is in part just a lack of
documentation, but I also think that this design just breaks easily.


Re. documentation: I didn't find any place where it is described what
should be wrapped in START_TARGET_SPECIFIC / END_TARGET_SPECIFIC. This
is important because if people don't know what to mark then new
target-specific things end up not put in the target-specific name
space. It is not obvious to me what is to be marked.

For example:
* why are create_mem_ref, addr_for_mem_ref, and maybe_fold_tmr wrapped
in START/END_TARGET_SPECIFIC in tree-flow.h, but tree_create_mem_ref
and get_address_description not?
* why is the whole df-problems.c file and a few other files target
specific? (Looks to me like these markers should be put fine-grained,
not on whole files)


Re. design breaks easily: There is no obvious way to check that all
functions that should be wrapped, are in fact wrapped. I suppose you
didn't find out what to wrap by trial and error. Do you have some
checking scripts that just list all functions that should be marked?

ISTM  it would be more robust to have a per-function-prototype marker
to signal that a function should be wrapped, and split off those
functions to separate files or a separate section in a file, instead
of wrapping the code in START/END_TARGET_SPECIFIC.

Otherwise, this patch is just waiting to be broken by ye random next
large patch (mem-ref2 merge, say) because nothing will catch missing
START/END_TARGET_SPECIFIC, reviewing is hard (you can't tell in a
patch whether a chunk is inside START/END_TARGET_SPECIFIC), and that's
all assuming people know what should be wrapped in the first place
(documentation).

Ciao!
Steven