1. I don't agree with "this patch must be updated". I don't see a bug here.
A compatibility (behavior-wise) with other compilers, may be
2. I'd like to see some example where you don't accidentally "dfe" some
functions that cannot be deleted. IOWs, I don't believe you thought about
this problem and understood what the underlying condition before a certain
function can be deleted. You just looked at what gcc or others behavior and
turn on/off some global flag. This is dangerous and not my recommended way
to change code
Sun
On Mon, Nov 21, 2011 at 11:21 AM, Gang Yu <yugang...@gmail.com> wrote:
> Sun,
>
> Thanks for the input.
>
> 'non-callable' should be right called 'non-called', means the function
> marked MARK_DELETED(deletable) by function Mark_Deletable_Funcs at
> ipa_cg.cxx. Informally, those functions unreachable by the call-graph can
> be marked 'DELETABLE' or 'non-called'.
>
> I think you refer IPA means IPA general, you are right. lwinline
> (inliner) is multiplexing part of the IPA general. lwinline does *the DFE*
> in inline.cxx before perform_inline:
>
> in inline.cxx:
> 1058BOOL
> 1059Inliner(char* input_name, char* output_name)
> 1060{
> 1188 if (IPA_Enable_DFE) {
> 1189#ifdef _LIGHTWEIGHT_INLINER
> 1190 if (!(INLINE_Inlined_Pu_Call_Graph ||
> INLINE_Inlined_Pu_Call_Graph2))
> 1191#endif // _LIGHTWEIGHT_INLINER
> 1192 {
> 1193 set_timer();
> 1194
> 1195 Total_Prog_Size = Orig_Prog_Weight -
> Eliminate_Dead_Func(FALSE);
> 1196
> 1197 get_timer(PHASE_DFE);
> 1198
> 1199 }
> 1200 }
> 1212 Perform_inlining();
> My suggested change does not affect IPA, inline_driver.cxx is not IPA
> part, and changes in ipa_cg.cxx(bug838), inline.cxx are enclosed
> in _LIGHTWEIGHT_INLINER.
>
> The suggested patch does not affect the inline behaviour, control should
> not go real inline performed, for those functions inlined then deleted, the
> control should never go there.
>
> This patch definitely should be updated, but I post out to see comments on
> the idea. the mentioned DFE and real inline should be departured.
>
> Regards
> Gang
>
>
> On Mon, Nov 21, 2011 at 7:50 AM, Sun Chan <sun.c...@gmail.com> wrote:
>
>> it's not clear what you meant by 'non-callable". please educate me.
>>
>> your change probably match the gcc behavior, but it's not clear it will
>> actually accomplish what you want as "DFE" behavior. I don't believe
>> O0/1/2/3 does no dfe, only IPA does. May be you can show me which part of
>> compiler will do DFE besides IPA? Even if dfe is performed, "inlined" does
>> not imply the function body can be removed, it's a lot more complicated
>> than "inlined"
>> Sun
>>
>> On Sun, Nov 20, 2011 at 7:59 PM, Gang Yu <yugang...@gmail.com> wrote:
>>
>>> Hi,
>>>
>>> could a gatekeeper help a review? any suggestions or comments are
>>> highly appreciated.
>>>
>>> This is an accidentally discovered issue when I review
>>> bug597<https://bugs.open64.net/show_bug.cgi?id=597>cases:
>>>
>>> cat bug921 <https://bugs.open64.net/show_bug.cgi?id=921>.c
>>> void static test(void)
>>> {
>>> asm("wrongthing");
>>> }
>>> yug@jupiter:~/work/bugs/bug921<https://bugs.open64.net/show_bug.cgi?id=921>>
>>> gcc -c bug921 <https://bugs.open64.net/show_bug.cgi?id=921>.c -O2
>>> yug@jupiter:~/work/bugs/bug921<https://bugs.open64.net/show_bug.cgi?id=921>>
>>> gcc -c bug921 <https://bugs.open64.net/show_bug.cgi?id=921>.c -O3
>>> yug@jupiter:~/work/bugs/bug921<https://bugs.open64.net/show_bug.cgi?id=921>>
>>> gcc -c bug921 <https://bugs.open64.net/show_bug.cgi?id=921>.c -O1
>>> yug@jupiter:~/work/bugs/bug921<https://bugs.open64.net/show_bug.cgi?id=921>>
>>> clang -c bug921 <https://bugs.open64.net/show_bug.cgi?id=921>.c -O3
>>> yug@jupiter:~/work/bugs/bug921<https://bugs.open64.net/show_bug.cgi?id=921>>
>>> clang -c bug921 <https://bugs.open64.net/show_bug.cgi?id=921>.c -O2
>>> yug@jupiter:~/work/bugs/bug921<https://bugs.open64.net/show_bug.cgi?id=921>>
>>> clang -c bug921 <https://bugs.open64.net/show_bug.cgi?id=921>.c -O1
>>> yug@jupiter:~/work/bugs/bug921<https://bugs.open64.net/show_bug.cgi?id=921>>
>>> clang -c bug921 <https://bugs.open64.net/show_bug.cgi?id=921>.c -O0
>>> yug@jupiter:~/work/bugs/bug921<https://bugs.open64.net/show_bug.cgi?id=921>>
>>> opencc -c bug921 <https://bugs.open64.net/show_bug.cgi?id=921>.c -O3
>>> yug@jupiter:~/work/bugs/bug921<https://bugs.open64.net/show_bug.cgi?id=921>>
>>> opencc -c bug921 <https://bugs.open64.net/show_bug.cgi?id=921>.c -O2
>>> /tmp/ccspin#.Efw754.s: Assembler messages:
>>> /tmp/ccspin#.Efw754.s:30: Error: no such instruction: `wrongthing'
>>> yug@jupiter:~/work/bugs/bug921<https://bugs.open64.net/show_bug.cgi?id=921>>
>>> opencc -c bug921 <https://bugs.open64.net/show_bug.cgi?id=921>.c -O1
>>> /tmp/ccspin#.9hLp6w.s: Assembler messages:
>>> /tmp/ccspin#.9hLp6w.s:30: Error: no such instruction: `wrongthing'
>>> yug@jupiter:~/work/bugs/bug921<https://bugs.open64.net/show_bug.cgi?id=921>>
>>> gcc -c bug921 <https://bugs.open64.net/show_bug.cgi?id=921>.c
>>> /tmp/cczPHnnc.s: Assembler messages:
>>> /tmp/cczPHnnc.s:11: Error: no such instruction: `wrongthing'
>>>
>>> obviously, opencc does not delete non-callable static functions at
>>> O1/O2, while other compilers do, clang even does more.
>>>
>>> Investigation shows driver launches the inliner according to wgen return
>>> value, i.e, if any function with INLINE attribute, inliner comes. At O3
>>> phase, gcc marks all static functions a GS_DECL_INLINE flag at spin tree,
>>> which makes the inliner always run. However, at O1/O2 level, it does not,
>>> so the inliner unfortunately ignored.
>>>
>>> The lwinliner actually does two things: DFE then inline. The right logic
>>> should be:
>>> - at O0 phase, if there are non-callable static functions declared with
>>> inline attribute, they should be deleted by DFE, no inline. futhur if there
>>> are inline arguments, call DFE and inline. Otherwise, silent.
>>> - at optimisation phases, if there are functions with inline attributed,
>>> do DFE and inline. Otherwise, we should also launch inliner for *DFE only*.
>>> At O3, gcc can help us always call inliner, at O1 or O2, we should open
>>> inliner ourselves and tell it only do DFE. since inline costs much.
>>>
>>> The suggested patch below:
>>> osprey/driver/phases.c -- 525b7d1..a47394a 100644
>>> --- a/osprey/driver/phases.c
>>> +++ b/osprey/driver/phases.c
>>> @@ -3197,6 +3197,7 @@ run_compiler (int argc, char *argv[])
>>> string_list_t *args;
>>> boolean inst_info_updated = FALSE;
>>> boolean cmd_line_updated = FALSE;
>>> + boolean dfe_only = FALSE;
>>> buffer_t rii_file_name;
>>> buffer_t ii_file_name;
>>> @@ -3215,8 +3216,14 @@ run_compiler (int argc, char *argv[])
>>> /* special case where the frontend decided that
>>> inliner should not be run */
>>> if (run_inline == FALSE && // bug 11325
>>> - phase_order[i] == P_inline)
>>> - continue;
>>> + phase_order[i] == P_inline) {
>>> + if (olevel == 0)
>>> + continue;
>>> + else {
>>> + dfe_only = TRUE;
>>> + run_inline = TRUE;
>>> + }
>>> + }
>>> if (is_matching_phase(get_phase_mask(phase_order[i]),
>>> P_any_ld)) {
>>> source_kind = S_o;
>>> @@ -3236,8 +3243,10 @@ run_compiler (int argc, char *argv[])
>>> 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"));
>>> + dfe_only = TRUE;
>>> }
>>> + if (dfe_only)
>>> + add_string(args,"-dfe-only");
>>> copy_phase_options (args, phase_order[i]);
>>> if (!cmd_line_updated &&
>>> @@ -3256,6 +3265,9 @@ run_compiler (int argc, char *argv[])
>>> if (has_current_errors()) break;
>>> run_phase (phase_order[i],
>>> get_full_phase_name(phase_order[i]),
>>> args);
>>> + /* clear dfe-only flag */
>>> + dfe_only = FALSE;
>>> +
>>> /* undefine the environment variable
>>> * DEPENDENCIES_OUTPUT after the pre-processor
>>> phase -
>>> * bug 386.
>>> osprey/ipa/inline/inline.cxx -- 7a2e8ad..d90abf7 100644
>>> --- a/osprey/ipa/inline/inline.cxx
>>> +++ b/osprey/ipa/inline/inline.cxx
>>> @@ -177,6 +177,9 @@ SCOPE* Orig_Scope_tab;
>>> extern IP_FILE_HDR_TABLE IP_File_header; // array of IP_FILE_HDR, which
>>> // holds per file information
>>> +#ifdef _LIGHTWEIGHT_INLINER
>>> +extern BOOL DFE_Only;
>>> +#endif // _LIGHTWEIGHT_INLINER
>>> #ifdef _LIGHTWEIGHT_INLINER
>>> AUX_PU_TAB Aux_Pu_Table;
>>> @@ -1204,7 +1207,10 @@ Inliner(char* input_name, char* output_name)
>>> fclose(f);
>>> #endif
>>> - Perform_inlining();
>>> +#ifdef _LIGHTWEIGHT_INLINER
>>> + if (!DFE_Only)
>>> + Perform_inlining();
>>> +#endif
>>> #ifdef _LIGHTWEIGHT_INLINER
>>> if (INLINE_Inlined_Pu_Call_Graph || INLINE_Inlined_Pu_Call_Graph2) {
>>> osprey/ipa/inline/inline_driver.cxx -- c56f5f5..7382653 100644
>>> --- a/osprey/ipa/inline/inline_driver.cxx
>>> +++ b/osprey/ipa/inline/inline_driver.cxx
>>> @@ -97,7 +97,10 @@ BOOL Verbose = FALSE;
>>> */
>>> SKIPLIST *IPA_Skip_List = NULL; /* List of skip options
>>> */
>>> -
>>> +/* The DFE_only flag to indicate whether call lwinline
>>> + * for DFE only.
>>> + */
>>> +BOOL DFE_Only = FALSE;
>>> /* ====================================================================
>>> *
>>> @@ -211,9 +214,11 @@ Process_Command_Line (INT argc, char **argv)
>>> break; /* ignore the -PHASE: option */
>>> case 'd': /* accept and ignore the -dsm flags
>>> */
>>> - if (strncmp (argv[i]+1, "dsm", 3) != 0)
>>> - ErrMsg ( EC_Unknown_Flag, argv[i][0], argv[i] );
>>> - break;
>>> + if (strncmp (argv[i] +1, "dfe-only", 8) == 0)
>>> + DFE_Only = TRUE;
>>> + else if (strncmp (argv[i]+1, "dsm", 3) != 0)
>>> + ErrMsg ( EC_Unknown_Flag, argv[i][0], argv[i] );
>>> + break;
>>> default: /* What's this? */
>>> ErrMsg ( EC_Unknown_Flag, argv[i][0], argv[i] );
>>>
>>> Regards
>>> Gang
>>>
>>>
>>> ------------------------------------------------------------------------------
>>> All the data continuously generated in your IT infrastructure
>>> contains a definitive record of customers, application performance,
>>> security threats, fraudulent activity, and more. Splunk takes this
>>> data and makes sense of it. IT sense. And common sense.
>>> http://p.sf.net/sfu/splunk-novd2d
>>> _______________________________________________
>>> Open64-devel mailing list
>>> Open64-devel@lists.sourceforge.net
>>> https://lists.sourceforge.net/lists/listinfo/open64-devel
>>>
>>>
>>
>
------------------------------------------------------------------------------
All the data continuously generated in your IT infrastructure
contains a definitive record of customers, application performance,
security threats, fraudulent activity, and more. Splunk takes this
data and makes sense of it. IT sense. And common sense.
http://p.sf.net/sfu/splunk-novd2d
_______________________________________________
Open64-devel mailing list
Open64-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/open64-devel
