Re: [PATCH 1b/9] Tilera (and Linux asm-generic) support for glibc

["Joseph S. Myers" <joseph at codesourcery dot com>]

On Fri, 4 Nov 2011, Chris Metcalf wrote:

> This patch is the disassembly support for Tile backtracing.  To be as
> robust as possible, we include support for discovering the frame base
> of the bottom-most stack frame by code inspection.  The files are mostly
> tables that pack down well when compiled.  The same code is present in
> the kernel to handle backtracing there.

Could you explain how unwinding works on your processors that is different 
from normal DWARF-2 unwinding (both .eh_frame and .debug_frame) used on 
other processors, and why instruction tables are needed here when no other 
processor that I know of needs them for unwinding?  I don't see anything 
obvious about this in your ABI documents (I also don't see any sign of ELF 
or DWARF ABIs at all in the documents linked from 
<http://www.tilera.com/scm/docs/index.html>).  The conventional approach 
for backtrace () on most targets is to use the IA64 version that uses the 
unwinder (_Unwind_Backtrace); some such as ARM may need their own variants 
on this implementation but still use the same basic strategy.  What are 
the advantages of this Tile-specific approach that needs large instruction 
tables supposed to be?

> +const struct tilegx_opcode tilegx_opcodes[334] =

As far as I can see the name tilegx_opcodes is not used at all outside 
this file, and so it should be static.

* In general, if an object (.o) in glibc contains a symbol that is not 
meant to be available outside that object (whether to other objects in 
glibc or to the user of glibc) then it should be static.

* If a symbol is an internal interface used by other objects within the 
same shared library but not meant to be available to users of glibc or 
other shared libraries within glibc, then it needs to be in the 
implementation namespace (starting "__") and should preferably be hidden 
visibility; this avoids interfering with the user's symbols when static 
linking.

* If a symbol is meant to be available to other shared libraries within 
glibc, but not directly to the user, it needs to be in the implementation 
namespace, symbol version GLIBC_PRIVATE, but not hidden visibility.

* Finally, symbols available to the user through glibc's shared libraries 
(including those in macro in glibc's installed headers, or used in objects 
such as libc_nonshared.a that are statically linked into user programs and 
shared libraries) need to be exported at symbol versions other than 
GLIBC_PRIVATE.  But if they are only used from macros and libc_nonshared 
rather than directly they should still be in the implementation namespace.

> +const unsigned short * const
> +tilegx_bundle_decoder_fsms[TILEGX_NUM_PIPELINE_ENCODINGS] =

static.

> +const struct tilegx_operand tilegx_operands[35] =

static.

> +int
> +parse_insn_tilegx(tilegx_bundle_bits bits,
> +                  unsigned long long pc,
> +                  struct tilegx_decoded_instruction
> +                  decoded[TILEGX_MAX_INSTRUCTIONS_PER_BUNDLE])

Hidden, implementation namespace.

> diff --git a/sysdeps/tile/tilepro/tile-desc.c b/sysdeps/tile/tilepro/tile-desc.c
> new file mode 100644
> index 0000000..1b066e5
> --- /dev/null
> +++ b/sysdeps/tile/tilepro/tile-desc.c

Same comments apply here as well.

The headers will also need corresponding updates.

-- 
Joseph S. Myers
joseph@codesourcery.com