priority is one issue, my second point, is more important, imho
Sun
On Mon, Nov 21, 2011 at 11:38 AM, Gang Yu <yugang...@gmail.com> wrote:
> Sun:
>
> If you don't think this behavior compatibility is essential, I am OK
> with that. We can save time to do more relevant things, but I am still
> interested in this topic. Any comments or inputs are welcome.
>
> Thanks
>
> Regards
> Gang
>
>
> On Mon, Nov 21, 2011 at 11:27 AM, Sun Chan <sun.c...@gmail.com> wrote:
>
>> 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
