Hi Jian-Xin,

Thanks for looking at the changes.

1. Since sometimes the names are constructed using sprintf, I don't
see how to make comp_name const.  We could allocate fixed-length
buffers if we want to avoid the strdup() and the leak with something
like:

    static char comp_name[MAX_LISTING_OPERANDS][MAX_COMP_NAME_LEN];

2. I thought the old declaration was confusing by being unusual:

    const char * const isa_exec_type_format = "...";

My change is to use the common C idiom for string constants:

    const char isa_exec_type_format[] = "...";

This form is preferred because it avoids introducing an unnecessary pointer.

-David Coakley / AMD Open Source Compiler Engineering

On Mon, Oct 11, 2010 at 11:22 AM, Jian-Xin Lai <laij...@gmail.com> wrote:
> Some comments:
> 1. In osprey/common/targ_info/generate/isa_print_gen.cxx(line 188) and
> osprey/common/targ_info/generate/isa_pack_gen.cxx (line 243):
> I wonder it's better to change the type of comp_name from char* to const
> char* so that we don't need to call strdup on string literal.
>
> 2. In osprey/common/targ_info/generate/isa_bundle_gen.cxx (line 349)
> It looks like changing the type of isa_exec_type_format from pointer to
> array is unnecessary.
>
> 2010/10/11 David Coakley <dcoak...@gmail.com>
>>
>> Hi Open64 gatekeepers,
>>
>> Has someone had a chance to review the attached cleanup patch?
>>
>> On Fri, Oct 1, 2010 at 4:26 PM, David Coakley <dcoak...@gmail.com> wrote:
>> > Hi all,
>> >
>> > I am submitting a proposal to eliminate the scheduling info DSOs and
>> > instead incorporate that info directly into the backend.  The primary
>> > motivation is portability.  The proposal is described in more detail
>> > in the attachment "no_si_dso.txt".
>> >
>> > Before making these changes, I thought it was a good idea to clean up
>> > the compilation warnings in targ_info and its generated code, and to
>> > fix some known portability problems (bugs 622, 624, 658).  The patch
>> > "ti_cleanup.diff" is a code cleanup patch that accomplishes those
>> > goals without making any changes in the current behavior.  The details
>> > are in the tentative log message, "log_msg.txt".
>> >
>> > Could a gatekeeper please review the patch?
>> >
>> > I have prototyped the proposed targ_info DSO changes for x8864.  I
>> > would like feedback from some of the gatekeepers and especially the
>> > maintainers for other targets -- I will need your help since the
>> > changes affect all targets.  Thanks,
>> >
>> > -David Coakley / AMD Open Source Compiler Engineering
>> >
>>
>>
>> ------------------------------------------------------------------------------
>> Beautiful is writing same markup. Internet Explorer 9 supports
>> standards for HTML5, CSS3, SVG 1.1,  ECMAScript5, and DOM L2 & L3.
>> Spend less time writing and  rewriting code and more time creating great
>> experiences on the web. Be a part of the beta today.
>> http://p.sf.net/sfu/beautyoftheweb
>> _______________________________________________
>> Open64-devel mailing list
>> Open64-devel@lists.sourceforge.net
>> https://lists.sourceforge.net/lists/listinfo/open64-devel
>>
>
>
>
> --
> Regards,
> Lai Jian-Xin
>

------------------------------------------------------------------------------
Beautiful is writing same markup. Internet Explorer 9 supports
standards for HTML5, CSS3, SVG 1.1,  ECMAScript5, and DOM L2 & L3.
Spend less time writing and  rewriting code and more time creating great
experiences on the web. Be a part of the beta today.
http://p.sf.net/sfu/beautyoftheweb
_______________________________________________
Open64-devel mailing list
Open64-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/open64-devel

Reply via email to