Hi Michael, All these items look good to us. Yong-Chong will check-in his previous patch soon. Thank you very much.
2011/11/1 Lai, Michael <michael....@amd.com>: > In the interest of getting Release 5.0 out, allow me to request the following > for this patch: > > 1. I really want this patch to be part of the 5.0 Release. > 2. Yongchong, this fix looks good. Also, in our private e-mail exchanges, > you indicated to me that this patch passed all regression and application > testing, so unless someone objects, please check this patch into the trunk. > This will be the last patch to be included in the 5.0 branch. > 3. Jian-Xin, you have proposed that instead of doing memcmp, compare st1 and > st2 field by field. Yongchong, please consider/discuss his proposal after > your check-in. (In other words, I suggest that we defer Jian-Xin's proposal > until *post* 5.0 Release.) > > Thanks, > Michael Lai > > [PS: I am working on the 5.0 branch as we speak. Wait for my next e-mail.] > > -----Original Message----- > From: Wu Yongchong [mailto:wuyongch...@gmail.com] > Sent: Sunday, October 30, 2011 7:47 PM > To: Sun Chan > Cc: open64-devel > Subject: Re: [Open64-devel] [open64-devel] Code review request for bug 883 > [IPA] > > Hi, > what about this, I change the position of the comment to top of the class > member definition > > Index: osprey/common/com/symtab_defs.h > =================================================================== > --- osprey/common/com/symtab_defs.h (revision 3775) > +++ osprey/common/com/symtab_defs.h (working copy) > @@ -223,6 +223,8 @@ > class ST > { > public: > + // after add new member, Make sure to update function > eq_const_st::operator() > + // in file ipc_symtab_merge.cxx > union { > STR_IDX name_idx; // index to the name string > TCON_IDX tcon; // constant value > Index: osprey/ipa/common/ipc_symtab_merge.cxx > =================================================================== > --- osprey/ipa/common/ipc_symtab_merge.cxx (revision 3775) > +++ osprey/ipa/common/ipc_symtab_merge.cxx (working copy) > @@ -159,7 +159,7 @@ > struct eq_const_st > { > bool operator() (const ST* st1, const ST* st2) const { > - return memcmp (st1, st2, sizeof(ST) - sizeof(ST_IDX) * 2 - > sizeof(TY_IDX)) == 0; > + return memcmp (st1, st2, sizeof(ST) - sizeof(ST_IDX) * 2 - > sizeof(TY_IDX) - sizeof(mUINT32)) == 0; > } > }; > > > On Mon, Oct 31, 2011 at 7:09 AM, Sun Chan <sun.c...@gmail.com> wrote: >> your comment should be at where future new member may be added. >> sun >> >> On Sun, Oct 30, 2011 at 11:01 PM, Wu Yongchong <wuyongch...@gmail.com> >> wrote: >>> >>> Hi, all >>> Can a gate keeper help review this patch. >>> See https://bugs.open64.net/show_bug.cgi?id=883 for reference. >>> Revision 3760 add a new member "mINT32 line" to structure ST, it >>> cause the function eq_const_st::operator() get wrong return value, >>> then make the symbol merge process in IPA not work. following is the >>> patch. I add a common to make sure anyone who add new member to >>> structure ST also update function eq_const_st::operator() >>> >>> Index: osprey/common/com/symtab_defs.h >>> =================================================================== >>> --- osprey/common/com/symtab_defs.h (revision 3776) >>> +++ osprey/common/com/symtab_defs.h (working copy) >>> @@ -261,6 +261,8 @@ >>> >>> mUINT32 line; // The line num where define the sym in >>> the source file. >>> >>> + // Make sure to update function eq_const_st::operator() in file >>> ipc_symtab_merge.cxx >>> + >>> // operations >>> >>> ST () {Fail_FmtAssertion("ST default constructor must not be >>> called.");} >>> Index: osprey/ipa/common/ipc_symtab_merge.cxx >>> =================================================================== >>> --- osprey/ipa/common/ipc_symtab_merge.cxx (revision 3776) >>> +++ osprey/ipa/common/ipc_symtab_merge.cxx (working copy) >>> @@ -159,7 +159,7 @@ >>> struct eq_const_st >>> { >>> bool operator() (const ST* st1, const ST* st2) const { >>> - return memcmp (st1, st2, sizeof(ST) - sizeof(ST_IDX) * 2 - >>> sizeof(TY_IDX)) == 0; >>> + return memcmp (st1, st2, sizeof(ST) - sizeof(ST_IDX) * 2 - >>> sizeof(TY_IDX) - sizeof(mUINT32)) == 0; >>> } >>> }; >>> >>> -- >>> yongchong >>> >>> >>> --------------------------------------------------------------------- >>> --------- Get your Android app more play: Bring it to the BlackBerry >>> PlayBook in minutes. BlackBerry App World™ now supports >>> Android™ Apps for the BlackBerry® PlayBook™. Discover >>> just how easy and simple it is! http://p.sf.net/sfu/android-dev2dev >>> _______________________________________________ >>> Open64-devel mailing list >>> Open64-devel@lists.sourceforge.net >>> https://lists.sourceforge.net/lists/listinfo/open64-devel >> >> > > > > -- > yongchong > > ------------------------------------------------------------------------------ > Get your Android app more play: Bring it to the BlackBerry PlayBook in > minutes. BlackBerry App World™ now supports Android™ Apps for the > BlackBerry® PlayBook™. Discover just how easy and simple it is! > http://p.sf.net/sfu/android-dev2dev > > _______________________________________________ > Open64-devel mailing list > Open64-devel@lists.sourceforge.net > https://lists.sourceforge.net/lists/listinfo/open64-devel > > > > ------------------------------------------------------------------------------ > Get your Android app more play: Bring it to the BlackBerry PlayBook > in minutes. BlackBerry App World™ now supports Android™ Apps > for the BlackBerry® PlayBook™. Discover just how easy and simple > it is! http://p.sf.net/sfu/android-dev2dev > _______________________________________________ > Open64-devel mailing list > Open64-devel@lists.sourceforge.net > https://lists.sourceforge.net/lists/listinfo/open64-devel > -- Regards, Lai Jian-Xin ------------------------------------------------------------------------------ RSA® Conference 2012 Save $700 by Nov 18 Register now http://p.sf.net/sfu/rsa-sfdev2dev1 _______________________________________________ Open64-devel mailing list Open64-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/open64-devel
