Thanks! Suneel
I consider it changes the O0 inline behavior for the backend components to
keep up with gnu. I am not sure whether other platforms are interested in
such change, just like in the ZDL patch, I introduce __attribute__((weak))
was not welcome, so I add a macro.
If it is reasonable for all contexts, I will remove the macro.
Regards
Gang
On Fri, Oct 28, 2011 at 9:53 PM, Suneel Jain <suneel.j...@gmail.com> wrote:
> Can this change be made unconditionally instead of putting it under
> "GNU_COMPATIBLE" ?
> Why is this change not reasonable for all contexts ?
> Adding this macro definition and use just seems added complexity.
>
> - Suneel
>
>
> On Thu, Oct 27, 2011 at 7:58 PM, Gang Yu <yugang...@gmail.com> wrote:
> > The patch is now ready, reviewed by Fred Chow and discussed with
> pathscale
> > guys. Thanks
> >
> > besides fixing bug838, the patch also enhances the gnu compatibility:
> > (1). enables -O0 -inline now
> > (2). does not delete non-callable static functions under O0
> > (3). bug838-1 to bug838-6 at the open64.net regression test suite prove
> > that.
> >
> > we also provide a _GNU_COMPATIBLE macro to identify the compiler is gnu
> > compatible one. For other backends with no gnu compatible interest, you
> can
> > set
> > BUILD_GNU_COMPATIBLE=FALSE in Makefile.gsetup.in
> >
> > Any comments please let us know.
> >
> > If no one objects, I will check the patch in tomorrow.
> >
> > Regards
> > Gang
> >
> > below is the patch:
> >
> > diff --git a/osprey/Makefile.gsetup.in b/osprey/Makefile.gsetup.in
> > index b7df96c..c987f08 100644
> > --- a/osprey/Makefile.gsetup.in
> > +++ b/osprey/Makefile.gsetup.in
> > @@ -76,6 +76,7 @@
> > # BUILD_OS = specifies OS (LINUX only right now)
> > # BUILD_HOST = specifies host that are building on
> > (MIPS/IA32/IA64/SL)
> > # [defaults to $(BUILD_ARCH)].
> > +# BUILD_GNU_COMPATIBLE = specifies whether to be gnu compatible
> > #
> > # Default values for BUILD_* parameters are set here with values from
> the
> > # configure script. BUILD_BASE is not set by configure but must be set
> > @@ -156,6 +157,9 @@ endif
> > ifndef GCC_DIR
> > GCC_DIR = @GCC_DIR@
> > endif
> > +ifndef BUILD_GNU_COMPATIBLE
> > + BUILD_GNU_COMPATIBLE = YES
> > +endif
> >
> > # For cross compilers, there are really 3 separate variables:
> > # the build host (machine you are building on),
> > @@ -664,6 +668,9 @@ STD_COMPILE_OPTS += -DKEY -DOSP_OPT -DPATHSCALE_MERGE
> > -DPSC_TO_OPEN64 $(LOCAL_CF
> > ifeq ($(BUILD_TYPE), SHARED)
> > STD_COMPILE_OPTS += -DSHARED_BUILD
> > endif
> > +ifeq ($(BUILD_GNU_COMPATIBLE), YES)
> > +STD_COMPILE_OPTS += -D_GNU_COMPATIBLE
> > +endif
> >
> > #
> > # Assign STD_C++_OPTS here
> > Modified osprey/driver/phases.c
> > diff --git a/osprey/driver/phases.c b/osprey/driver/phases.c
> > index 8451756..b67f1dd 100644
> > --- a/osprey/driver/phases.c
> > +++ b/osprey/driver/phases.c
> > @@ -3228,6 +3228,18 @@ run_compiler (int argc, char *argv[])
> > } else {
> > args = init_string_list();
> > add_file_args_first (args, phase_order[i]); //
> bug
> > 6874
> > +#ifdef _GNU_COMPATIBLE
> > + if (phase_order[i] == P_inline &&
> > + run_inline == TRUE &&
> > + olevel == 0 &&
> > + !(option_was_seen(O_INLINE_) ||
> > + option_was_seen(O_INLINE) ||
> > + option_was_seen(O_inline) ||
> > + option_was_seen(O_finline) ||
> > + option_was_seen(O_finline_functions))) {
> > + prepend_option_seen
> > (add_string_option(O_INLINE_, "none"));
> > + }
> > +#endif
> > copy_phase_options (args, phase_order[i]);
> >
> > if (!cmd_line_updated &&
> > Modified osprey/driver/special_options.c
> > diff --git a/osprey/driver/special_options.c
> > b/osprey/driver/special_options.c
> > index 65f3092..fed6ed9 100644
> > --- a/osprey/driver/special_options.c
> > +++ b/osprey/driver/special_options.c
> > @@ -365,8 +365,14 @@ add_special_options (void)
> > * -g to change the generated code so we leave it off always.
> > * See bugs 1917 and 7595.
> > */
> > + /*
> > + open64.net bug838, if defined _GNU_COMPATIBLE then, we
> > + open inline for O0.
> > + */
> > +#ifndef _GNU_COMPATIBLE
> > if (olevel == 0 && inline_t == UNDEFINED)
> > inline_t = FALSE;
> > +#endif
> >
> > /* In the SGI world, -g3 says to emit crippled debug info for
> use
> > * with optimized code. In the GNU/Pathscale world, -g3 says to
> emit
> > Modified osprey/ipa/inline/inline_driver.cxx
> > diff --git a/osprey/ipa/inline/inline_driver.cxx
> > b/osprey/ipa/inline/inline_driver.cxx
> > index 77704bc..5060e8b 100644
> > --- a/osprey/ipa/inline/inline_driver.cxx
> > +++ b/osprey/ipa/inline/inline_driver.cxx
> > @@ -196,6 +196,12 @@ Process_Command_Line (INT argc, char **argv)
> > Opt_Level = 2;
> > INLINE_Enable_Split_Common = FALSE;
> > break;
> > +#ifdef _GNU_COMPATIBLE
> > + case '0': /* setup O0 for lw_inline in gcc
> compatible
> > compiler*/
> > + Opt_Level = 0;
> > + INLINE_Enable_Split_Common = FALSE;
> > + break;
> > +#endif
> > default:
> > Opt_Level = 1;
> > INLINE_Enable_Split_Common = FALSE;
> > Modified osprey/ipa/main/analyze/ipa_cg.cxx
> > diff --git a/osprey/ipa/main/analyze/ipa_cg.cxx
> > b/osprey/ipa/main/analyze/ipa_cg.cxx
> > index 5b58f01..ce74695 100644
> > --- a/osprey/ipa/main/analyze/ipa_cg.cxx
> > +++ b/osprey/ipa/main/analyze/ipa_cg.cxx
> > @@ -1848,6 +1848,14 @@ Mark_Deletable_Funcs (NODE_INDEX v, DFE_ACTION
> > action, mUINT8 *visited)
> > node->Set_Undeletable();
> > action = MARK_USED;
> > visited[v] = VISITED_AND_KEEP;
> > +#if defined(_LIGHTWEIGHT_INLINER) && defined(_GNU_COMPATIBLE)
> > + /* O0 does not delete functions with no inline attrib */
> > + } else if (Opt_Level == 0 && ! node->Has_Inline_Attrib()) {
> > + node->Clear_Deletable();
> > + node->Set_Undeletable();
> > + action = MARK_USED;
> > + visited[v] = VISITED_AND_KEEP;
> > +#endif
> > } else if (visited[v] == 0)
> > visited[v] = VISITED_BUT_UNDECIDED;
> > break;
> >
> >
> > On Wed, Oct 12, 2011 at 1:38 PM, Gang Yu <yugang...@gmail.com> wrote:
> >>
> >> Sorry for the duplication effort on this bug, before my patch, yongchong
> >> had also submitted a patch to the list.
> >>
> >> I personally think my patch will be better, since patch from build
> system
> >> does not solve the compability problem with gnu and -inline will do
> really
> >> inline which is not a desired behavior under O0. we can add options with
> >> -INLINE=on, -INLINE:none to work around, but this does not solve
> the root
> >> cause.
> >>
> >> Could gatekeeper on IPA or global gatekeeper help review this? it is a
> >> major severity bug in bugzilla.
> >>
> >> Here is his original mail:
> >>
> >> Hi
> >> Can a gatekeeper help review this patch, it's a bug fixed for bug#838
> >> When building the gcc front end with opencc -g -O0, there is some
> >> dead functions that call other undefined functions. when building with
> >> gcc or opencc -O1 optimization above, the compiler will eliminated the
> >> dead function. But when building with opencc -O0, dead function would
> >> be kept, this fact will cause the undefined reference error.
> >> This patch will add options -INLINE when build using open64 to build
> >> the front end with DEBUG optimization. With -O0 -INLINE, the open64
> >> can eliminated the dead functions.
> >> Index: configure
> >> ===================================================================
> >> --- configure (revision 3737)
> >> +++ configure (working copy)
> >> @@ -1418,6 +1418,9 @@
> >> DEBUG|debug)
> >> BUILD_OPTIMIZE=DEBUG
> >> GCC_CONFIGURE_CFLAGS="-O0 -g -DIs_True_On"
> >> + if test "${BUILD_COMPILER}" = "OSP"; then
> >> + GCC_CONFIGURE_CFLAGS="${GCC_CONFIGURE_CFLAGS} -INLINE"
> >> + fi
> >> ;;
> >> *)
> >> { { echo "$as_me:$LINENO: error: \"BUILD_OPTIMIZE=$BUILD_OPTIMIZE
> >> is not supported\"" >&5
> >> Index: configure.ac
> >> ===================================================================
> >> --- configure.ac (revision 3737)
> >> +++ configure.ac (working copy)
> >> @@ -88,6 +88,9 @@
> >> DEBUG|debug)
> >> BUILD_OPTIMIZE=DEBUG
> >> GCC_CONFIGURE_CFLAGS="-O0 -g -DIs_True_On"
> >> + if test "${BUILD_COMPILER}" = "OSP"; then
> >> + GCC_CONFIGURE_CFLAGS="${GCC_CONFIGURE_CFLAGS} -INLINE"
> >> + fi
> >> ;;
> >> *)
> >> AC_MSG_ERROR(["BUILD_OPTIMIZE=$BUILD_OPTIMIZE is not supported"])
> >> --
> >> yongchong
> >>
> >> Regards
> >> Gang
> >>
> >>
> >> On Sat, Oct 8, 2011 at 9:26 AM, Gang Yu <yugang...@gmail.com> wrote:
> >>>
> >>> Hi,
> >>>
> >>> Could a gatekeeper please help review the fix for bug838?
> >>> http://bugs.open64.net/show_bug.cgi?id=838
> >>>
> >>> symptom:
> >>>
> >>> opencc could not compile gcc for debug build(O0), a cutted down case
> as
> >>> below:
> >>>
> >>> int i;
> >>> static __inline__ void
> >>> should_not_exist (void)
> >>> {
> >>> i=1;
> >>> }
> >>>
> >>> yug@jupiter:~/work/bugs/bug838> $TOOLROOT/bin/opencc -c case1.i -O0
> >>> yug@jupiter:~/work/bugs/bug838> readelf -s case1.o | grep
> >>> should_not_exist
> >>> 4: 0000000000000000 21 FUNC LOCAL DEFAULT 1
> >>> should_not_exist
> >>> yug@jupiter:~/work/bugs/bug838> gcc -c -O0 case1.i
> >>> yug@jupiter:~/work/bugs/bug838> readelf -s case1.o | grep
> >>> should_not_exist
> >>>
> >>> Since opencc does not do inline on O0(non-ipa) option, so no deletion
> >>> for should_not_exit, this incompatibility causes the break.
> >>>
> >>> Solution:
> >>>
> >>> open inline for O0, but only do dead function elimination
> >>> for unreachable functions with inline attribute.
> >>>
> >>> Patch:
> >>>
> >>> --- a/osprey/driver/special_options.c
> >>> +++ b/osprey/driver/special_options.c
> >>> @@ -360,13 +360,6 @@ add_special_options (void)
> >>> turn_down_opt_level(0, "-g changes optimization to -O0
> >>> since no optimization level is specified");
> >>> }
> >>> - /* Turn off inlining when compiling -O0. We definitly want
> >>> - * this off when compiling with -g -O0, but we don't want
> >>> - * -g to change the generated code so we leave it off always.
> >>> - * See bugs 1917 and 7595.
> >>> - */
> >>> - if (olevel == 0 && inline_t == UNDEFINED)
> >>> - inline_t = FALSE;
> >>> /* In the SGI world, -g3 says to emit crippled debug info for
> >>> use
> >>> * with optimized code. In the GNU/Pathscale world, -g3 says to
> >>> emit
> >>> Modified osprey/ipa/inline/inline.cxx
> >>> diff --git a/osprey/ipa/inline/inline.cxx
> b/osprey/ipa/inline/inline.cxx
> >>> index 7a2e8ad..a41e779 100644
> >>> --- a/osprey/ipa/inline/inline.cxx
> >>> +++ b/osprey/ipa/inline/inline.cxx
> >>> @@ -635,6 +635,9 @@ Perform_inlining()
> >>> #endif // _LIGHTWEIGHT_INLINER
> >>> Write_inline_succ_pu();
> >>> } else {
> >>> +#ifdef _LIGHTWEIGHT_INLINER
> >>> + if (Opt_Level !=0) /* O0 does not do real inline. */
> >>> +#endif
> >>> Inline_callees_into_caller(caller);
> >>> #ifdef DEBUG_SYMTAB
> >>> Scope_tab = caller->Scope();
> >>> Modified osprey/ipa/inline/inline_driver.cxx
> >>> diff --git a/osprey/ipa/inline/inline_driver.cxx
> >>> b/osprey/ipa/inline/inline_driver.cxx
> >>> index 77704bc..4aff837 100644
> >>> --- a/osprey/ipa/inline/inline_driver.cxx
> >>> +++ b/osprey/ipa/inline/inline_driver.cxx
> >>> @@ -196,6 +196,10 @@ Process_Command_Line (INT argc, char **argv)
> >>> Opt_Level = 2;
> >>> INLINE_Enable_Split_Common = FALSE;
> >>> break;
> >>> + case '0': /* setup O0 for lw_inline */
> >>> + Opt_Level = 0;
> >>> + INLINE_Enable_Split_Common = FALSE;
> >>> + break;
> >>> default:
> >>> Opt_Level = 1;
> >>> INLINE_Enable_Split_Common = FALSE;
> >>> Modified osprey/ipa/main/analyze/ipa_cg.cxx
> >>> diff --git a/osprey/ipa/main/analyze/ipa_cg.cxx
> >>> b/osprey/ipa/main/analyze/ipa_cg.cxx
> >>> index 5b58f01..4638977 100644
> >>> --- a/osprey/ipa/main/analyze/ipa_cg.cxx
> >>> +++ b/osprey/ipa/main/analyze/ipa_cg.cxx
> >>> @@ -1848,6 +1848,14 @@ Mark_Deletable_Funcs (NODE_INDEX v, DFE_ACTION
> >>> action, mUINT8 *visited)
> >>> node->Set_Undeletable();
> >>> action = MARK_USED;
> >>> visited[v] = VISITED_AND_KEEP;
> >>> +#ifdef _LIGHTWEIGHT_INLINER
> >>> + /* O0 does not delete functions with no inline attrib */
> >>> + } else if (Opt_Level == 0 && ! node->Has_Inline_Attrib()) {
> >>> + node->Clear_Deletable();
> >>> + node->Set_Undeletable();
> >>> + action = MARK_USED;
> >>> + visited[v] = VISITED_AND_KEEP;
> >>> +#endif
> >>> } else if (visited[v] == 0)
> >>> visited[v] = VISITED_BUT_UNDECIDED;
> >>> break;
> >>>
> >>>
> >>> Regards
> >>> Gang
> >>
> >
> >
> >
> ------------------------------------------------------------------------------
> > The demand for IT networking professionals continues to grow, and the
> > demand for specialized networking skills is growing even more rapidly.
> > Take a complimentary Learning@Cisco Self-Assessment and learn
> > about Cisco certifications, training, and career opportunities.
> > http://p.sf.net/sfu/cisco-dev2dev
> > _______________________________________________
> > Open64-devel mailing list
> > Open64-devel@lists.sourceforge.net
> > https://lists.sourceforge.net/lists/listinfo/open64-devel
> >
> >
>
------------------------------------------------------------------------------
The demand for IT networking professionals continues to grow, and the
demand for specialized networking skills is growing even more rapidly.
Take a complimentary Learning@Cisco Self-Assessment and learn
about Cisco certifications, training, and career opportunities.
http://p.sf.net/sfu/cisco-dev2dev
_______________________________________________
Open64-devel mailing list
Open64-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/open64-devel
