Comparing the source tree is very slow and inefficient, especially when there is no good diff tools. I think there are some other advantages to use macros. For example, when one navigates the code in osprey-gcc, he can easily know what code belongs to original GCC and what are added by Open64.
2010/8/17 David Coakley <dcoak...@gmail.com>: > Personally, I would not trust manually added ifdef's to reveal all the > Open64 modifications to the front-end. A reliable way to see the > differences is to download the stock gcc-4.2.0 sources > (http://mirrors.kernel.org/gnu/gcc/gcc-4.2.0) and compare the trees. > I don't really see the value of using KEY or any other ifdef to mark > change regions. > > I do agree that the changes should be enclosed by a check on > 'flag_spin_file'. In my last set of changes to osprey-gcc-4.2.0, I > made sure that the resulting gcc behaved the same as the stock > gcc-4.2.0 when '-spinfile' was not used on the command line. > > -David Coakley / AMD Open Source Compiler Engineering > > On Tue, Aug 17, 2010 at 4:00 PM, Mike Murphy <mmur...@nvidia.com> wrote: >> I agree that we shouldn't use KEY anymore (it has an obscure history that is >> no longer relevant). Better to use something like #ifdef OPEN64 in the gcc >> files. >> >> -----Original Message----- >> From: Suneel Jain [mailto:suneel.j...@gmail.com] >> Sent: Tuesday, August 17, 2010 3:56 PM >> To: Jian-Xin Lai >> Cc: open64-devel@lists.sourceforge.net >> Subject: Re: [Open64-devel] duplicate_decls bug >> >> I agree these changes should be under an ifdef so we can easily identify >> Open64 specific changes in the gcc directories. This makes it >> easier to merge with future versions of gcc frontends. >> >> However, I think we should use a macro name that conveys this more >> explicitly instead of using "#ifdef KEY". Do we have a convention for >> names of such macros and where they should be defined ? >> >> Some possibilities for the macros name: >> >> OPEN64_SPIN >> GCC_SPIN_GEN >> >> Comments ? >> >> - Suneel >> >> >> >> On Mon, Aug 16, 2010 at 3:43 PM, Jian-Xin Lai <laij...@gmail.com> wrote: >>> I think these changes should be enclosed by some macro ( shall we use >>> the #ifdef KEY? ). Also, these changes should be only applied under >>> "if (flag_spin_file)". >>> For example: >>> >>> if (flag_spin_file) { >>> TREE_TO_TRANSLATED_GS(olddecl) = TREE_TO_TRANSLATED_GS(newdecl); >>> FULLY_TRANSLATED_TO_GS(olddecl) = FULLY_TRANSLATED_TO_GS(newdecl); >>> } >>> >>> 2010/8/15 Chen, Rui (Roger, TSG-GDCC-SH) <rui.c...@hp.com>: >>>> Hi all, >>>> >>>> >>>> >>>> This is the fix for bug 582 (https://bugs.open64.net/show_bug.cgi?id=582). >>>> Can gatekeeper help to review it? >>>> >>>> >>>> >>>> I attached the test case (min.i) and patch (582.patch). You can reproduce >>>> the problem with "openCC -O0 -g -c min.i". >>>> >>>> >>>> >>>> This bug caused by merge user declared "log" function with built-in "log" >>>> function. The front-end forgets to copy GSPIN related fields and >>>> declaration >>>> assembler name. >>>> >>>> >>>> >>>> If you are interested in the analysis process, here it is: >>>> >>>> >>>> >>>> Breakpoints you need to setup: >>>> >>>> b name-lookup.c:615 if strcmp(name.identifier.id.str, "log") == 0 >>>> >>>> b passes.c:148 >>>> >>>> >>>> >>>> Compiler will initial built-in functions first, so those functions declared >>>> in osprey-gcc-4.2.0/gcc/builtins.def will be translated to gcc tree. And >>>> built-in "log" will be translated twice, one into global namespace, one >>>> into >>>> std namespace. >>>> >>>> >>>> >>>> Then the second breakpoint is triggered. This method checks whether the >>>> declared function has "alias" attribute or not. If the answer is yes, it >>>> will translate gcc tree to open64 gspin tree: >>>> >>>> >>>> >>>> #ifdef KEY >>>> >>>> /* Put aliases into the list of decls emitted by g++ so that we can >>>> >>>> * iterate through the list when expanding aliases to WHIRL. An >>>> alias >>>> >>>> * can be expanded only if its target, which can be another alias, >>>> is >>>> >>>> * expanded. Bug 4393. */ >>>> >>>> if (flag_spin_file) >>>> >>>> gspin_gxx_emits_decl(decl); >>>> >>>> #endif >>>> >>>> >>>> >>>> In this test case, it declares: >>>> >>>> >>>> >>>> extern __typeof(pthread_once) __gthrw_pthread_once >>>> __attribute__ ((__weakref__("pthread_once"))); >>>> >>>> >>>> >>>> __weakref__ means "alias" attribute, so compiler start translation at this >>>> time. At this time, only built-in log can be found by compiler. >>>> >>>> >>>> >>>> After that, compiler parsed user defined "log". It then find out we have >>>> already defined a "log" in the global namespace. (name-lookup.c, line 623) >>>> So it tries to merge the declarations. (duplicate_decls function in decl.c) >>>> >>>> >>>> >>>> I patched this file, to let it also copy gspin related fields and >>>> declaration assembler name. >>>> >>>> >>>> >>>> Then after compiler parsed the whole file, it will translate gcc tree to >>>> gspin tree again. Without the patch, it just re-use the built-in log gspin >>>> tree. With this patch, it will rebuild the log gspin tree using the new log >>>> declaration. >>>> >>>> >>>> >>>> Regards, >>>> >>>> Roger >>>> >>>> ------------------------------------------------------------------------------ >>>> This SF.net email is sponsored by >>>> >>>> Make an app they can't live without >>>> Enter the BlackBerry Developer Challenge >>>> http://p.sf.net/sfu/RIM-dev2dev >>>> _______________________________________________ >>>> Open64-devel mailing list >>>> Open64-devel@lists.sourceforge.net >>>> https://lists.sourceforge.net/lists/listinfo/open64-devel >>>> >>>> >>> >>> >>> >>> -- >>> Regards, >>> Lai Jian-Xin >>> >>> ------------------------------------------------------------------------------ >>> This SF.net email is sponsored by >>> >>> Make an app they can't live without >>> Enter the BlackBerry Developer Challenge >>> http://p.sf.net/sfu/RIM-dev2dev >>> _______________________________________________ >>> Open64-devel mailing list >>> Open64-devel@lists.sourceforge.net >>> https://lists.sourceforge.net/lists/listinfo/open64-devel >>> >> >> ------------------------------------------------------------------------------ >> This SF.net email is sponsored by >> >> Make an app they can't live without >> Enter the BlackBerry Developer Challenge >> http://p.sf.net/sfu/RIM-dev2dev >> _______________________________________________ >> Open64-devel mailing list >> Open64-devel@lists.sourceforge.net >> https://lists.sourceforge.net/lists/listinfo/open64-devel >> ----------------------------------------------------------------------------------- >> This email message is for the sole use of the intended recipient(s) and may >> contain >> confidential information. Any unauthorized review, use, disclosure or >> distribution >> is prohibited. If you are not the intended recipient, please contact the >> sender by >> reply email and destroy all copies of the original message. >> ----------------------------------------------------------------------------------- >> >> ------------------------------------------------------------------------------ >> This SF.net email is sponsored by >> >> Make an app they can't live without >> Enter the BlackBerry Developer Challenge >> http://p.sf.net/sfu/RIM-dev2dev >> _______________________________________________ >> Open64-devel mailing list >> Open64-devel@lists.sourceforge.net >> https://lists.sourceforge.net/lists/listinfo/open64-devel >> > -- Regards, Lai Jian-Xin ------------------------------------------------------------------------------ This SF.net email is sponsored by Make an app they can't live without Enter the BlackBerry Developer Challenge http://p.sf.net/sfu/RIM-dev2dev _______________________________________________ Open64-devel mailing list Open64-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/open64-devel
