Hello Michael:
Thanks for the comments on ChangeLog. Modified ChangeLog is inlined below.
2014-05-13 Ajit Agarwal <ajit...@xilinx.com>
* config/microblaze/microblaze.c
(break_handler): New Declaration.
(microblaze_break_function_p,microblaze_is_break_handler) : New function.
(compute_frame_size): use of microblaze_break_function_p. Add the test
of break_handler.
(microblaze_function_prologue,microblaze_function_epilogue) : Add the
test of variable
break_handler.
(microblaze_globalize_label) : Add the test of break_handler.
* config/microblaze/microblaze.h (BREAK_HANDLER_NAME) : New macro
* config/microblaze/microblaze.md :
(*<optab>,<optab>_internal): Add microblaze_is_break_handler () test.
(call_internal1,call_value_intern) : Use of microblaze_break_function_p.
Use of SYMBOL_REF_DECL.
* config/microblaze/microblaze-protos.h
(microblaze_break_function_p,microblaze_is_break_handler) : New
Declaration.
* testsuite/gcc.target/microblaze/others/break_handler.c : New.
Thanks & Regards
Ajit
-----Original Message-----
From: Michael Eager [mailto:ea...@eagercon.com]
Sent: Tuesday, May 13, 2014 10:30 PM
To: Ajit Kumar Agarwal; gcc-patches@gcc.gnu.org
Cc: Vinod Kathail; Vidhumouli Hunsigida; Nagaraju Mekala
Subject: Re: [Patch,Microblaze]: Added Break Handler Support
On 05/13/14 02:14, Ajit Kumar Agarwal wrote:
> Hello Michael:
>
> The following patch is to handle Software and Hardware breaks in Microblaze
> Architecture.
> Deja GNU testcase does not have any regressions and the testcase attached
> passes through.
> Review comments are incorporated.
>
> Okay for trunk?
Just saying OK would only be appropriate if you had write access to the
repository.
> Added Break Handler support to incorporate the hardware and software
> break. The Break Handler routine will be generating the rtbd
> instruction. At the call point where the software breaks are generated with
> the instruction brki with register operand as r16.
>
> 2014-05-13 Ajit Agarwal <ajit...@xilinx.com>
>
> * config/microblaze/microblaze.c
> (microblaze_break_function_p,microblaze_is_break_handler) : New
>
> * config/microblaze/microblaze.h (BREAK_HANDLER_NAME) : New macro
>
> * config/microblaze/microblaze.md :
> Extended support for generation of brki instruction and rtbd instruction.
A better ChangeLog entry is
* config/microblaze/microblaze.md (*<optab>,<optab>_internal):
Add microblaze_is_break_handler () test.
Give specifics, naming functions, rather than making general comments.
As the ChangeLog standard says:
It’s important to name the changed function or variable in full.
Don’t abbreviate function or variable names, and don’t combine them.
Subsequent maintainers will often search for a function name to find
all the change log entries that pertain to it; if you abbreviate the
name, they won’t find it when they search.
Mention each place where there are changes. There should be a ChangeLog entry
for each non-trivial change.
Your patch made four significant changes to microblaze.md.
There appear to be several changes in microblaze.c, not just the definition of
the new functions as shown in your entry.
> * config/microblaze/microblaze-protos.h
> (microblaze_break_function_p,microblaze_is_break_handler) : New
> Declaration.
>
> * testsuite/gcc.target/microblaze/others/break_handler.c : New.
Thanks for the test case.
As mentioned previously, add documentation for _break_handler.
> diff --git a/gcc/config/microblaze/microblaze-protos.h
> b/gcc/config/microblaze/microblaze-protos.h
> index b03e9e1..f3cc099 100644
> --- a/gcc/config/microblaze/microblaze-protos.h
> +++ b/gcc/config/microblaze/microblaze-protos.h
Please include the patch only once, not both inline and again as an attachment.
--
Michael Eager ea...@eagercon.com
1960 Park Blvd., Palo Alto, CA 94306 650-325-8077
