On 03/14/2013 01:07 AM, David Holsgrove wrote:
Hi Michael,

-----Original Message-----
From: Michael Eager [mailto:ea...@eagerm.com]
Sent: Thursday, 14 March 2013 1:34 am
To: David Holsgrove
Cc: gcc-patches@gcc.gnu.org; Edgar Iglesias; John Williams; Vinod Kathail;
Vidhumouli Hunsigida; Nagaraju Mekala; Tom Shui
Subject: Re: [Patch, microblaze]: Add support for TLS in MicroBlaze

On 03/12/2013 01:47 PM, David Holsgrove wrote:
Add support for thread local storage (general dynamic and local dynamic
models) in MicroBlaze.


gcc/Changelog

2013-03-13  Edgar E. Iglesias <edgar.igles...@xilinx.com>
              David Holsgrove <david.holsgr...@xilinx.com>

[--snip--]

Signed-off-by: Edgar E. Iglesias <edgar.igles...@xilinx.com>
Signed-off-by: David Holsgrove <david.holsgr...@xilinx.com>

Hi David --

The patch is OK except for a number of minor formatting problems to meet GNU
standards.
- Comments are supposed to end with ".  */".
- Extra spacing around parens or between "!" and "TARGET...".
- && or || at start of line continuing a condition rather that at end of 
previous line
- Some places where the indenting is incorrect


Thanks for the review, we inherited some of the style deviations when basing off
of other archs TLS functions I'm afraid. I've fixed the formatting in v2 of the 
patch
attached.

Should this patch be combined with the other patch adding TLS checking to
configure?


I've combined both patches into the attached - looking back at the ARM TLS patch
it was submitted this way, so we'll follow precedence there if that’s okay with
you.

Updated Changelog would be;

2013-03-14 Edgar E. Iglesias <edgar.igles...@xilinx.com>
            David Holsgrove <david.holsgr...@xilinx.com>

  * configure.ac: Add MicroBlaze TLS support detection.
  * configure: Regenerate.
  * config/microblaze/microblaze-protos.h: (microblaze_cannot_force_const_mem,
    microblaze_tls_referenced_p, symbol_mentioned_p,
    label_mentioned_p): Add prototypes.
  * config/microblaze/microblaze.c (microblaze_address_type): Add ADDRESS_TLS
    and tls_reloc address types.
    (microblaze_address_info): Add tls_reloc.
    (TARGET_HAVE_TLS): Define.
    (get_tls_get_addr, microblaze_tls_symbol_p, microblaze_tls_operand_p_1,
     microblaze_tls_referenced_p, microblaze_cannot_force_const_mem,
     symbol_mentioned_p, label_mentioned_p, tls_mentioned_p, load_tls_operand,
     microblaze_call_tls_get_addr, microblaze_legitimize_tls_address): New 
functions.
    (microblaze_classify_unspec): Handle UNSPEC_TLS.
    (get_base_reg): Use microblaze_tls_symbol_p.
    (microblaze_classify_address): Handle TLS.
    (microblaze_legitimate_pic_operand): Use symbol_mentioned_p, 
label_mentioned_p
     and microblaze_tls_referenced_p.
    (microblaze_legitimize_address): Handle TLS.
    (microblaze_address_insns): Handle ADDRESS_TLS.
    (pic_address_needs_scratch): Handle TLS.
    (print_operand_address): Handle TLS.
    (microblaze_expand_prologue): Check TLS_NEEDS_GOT.
    (microblaze_expand_move): Handle TLS.
    (microblaze_legitimate_constant_p): Check microblaze_cannot_force_const_mem
     and microblaze_tls_symbol_p.
    (TARGET_CANNOT_FORCE_CONST_MEM): Define.
  * config/microblaze/microblaze.h (TLS_NEEDS_GOT): Define
    (PIC_OFFSET_TABLE_REGNUM): Set.
  * config/microblaze/linux.h (TLS_NEEDS_GOT): Define.
  * config/microblaze/microblaze.md (UNSPEC_TLS): Define.
    (addsi3, movsi_internal2, movdf_internal): Update constraints
  * config/microblaze/predicates.md (arith_plus_operand): Define
    (move_operand): Redefine as move_src_operand, check 
microblaze_tls_referenced_p.

Signed-off-by: Edgar E. Iglesias <edgar.igles...@xilinx.com>
Signed-off-by: David Holsgrove <david.holsgr...@xilinx.com>

Thanks for cleaning up the patch.  There were a few other places where I fixed
indent and other formatting issues.

Committed revision 196659.

--
Michael Eager    ea...@eagercon.com
1960 Park Blvd., Palo Alto, CA 94306  650-325-8077

Reply via email to