Hi David,
Please refer my comments in below.
2010/10/11 David Coakley <dcoak...@gmail.com>
> 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];
>
Convertion from char * (return type of strdup()) to const char * is safe.
Since we don't release the memory allocated by strdup, it looks your change
increases the memory leakage. The comp_name can be defined like:
static const char *comp_name[MAX_LISTING_OPERANDS];
>
> 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.
>
Since isa_exec_type_format is read only, we don't need to allocate space on
stack to hold the string. The second 'const' in original code ensures the
value of the pointer itself to be const.
>
> -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
> >
>
--
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
