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
