DFE and real inlining are really separate steps. I don't think we will
unsafely delete the non-deletable functions. The final patch is not ready,
and before it is ready, as serious engineering, we will do plenty of tests,
we will even dump the symbol table and compared it with the gcc's, clang's
results.
Before the final result ready, I don't think anything is impossible. As
paied employees, we just need to focus on the assigned jobs.
Thanks
Regards
Gang
On Mon, Nov 21, 2011 at 12:55 PM, Sun Chan <sun.c...@gmail.com> wrote:
> 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
