Thanks for the input, Murthy
whirl dump of your code at the beginning of CG_Generate_Code is:
I4INTCONST 4 (0x4)
I4STID 0 <2,2,size> T<4,.predef_I4,4> {line: 1/10}
I4I4LDID 0 <2,2,size> T<4,.predef_I4,4>
I4STID 0 <2,3,_temp__switch_index0> T<4,.predef_I4,4> {line: 1/11}
I4I4LDID 0 <2,3,_temp__switch_index0> T<4,.predef_I4,4>
I4INTCONST 4 (0x4)
I4I4EQ
TRUEBR L258 {line: 1/17}
Kid of last I4STID is I4I4LDID, my patch will not copy prop to TRUEBR, so
even with the new patch, opencc will still emit call to undefined fun at
O0. In this sense, it is compatible to gcc.
at VHO phase, there is no CFG present, handling and deleting the branch
will be something tricky......
correct me if I am wrong.
Thanks
Regards
Gang
On Sat, Mar 17, 2012 at 10:53 PM, Chandrasekhar Murthy <mur...@sgi.com>wrote:
> I agree.
> It would be easier to fix it in VHO.
>
> Modifying the code to
>
> int defined_fun()
> {
> }
> int main()
> {
> int t;
> int size = sizeof(t);
> switch(size)
> {
> case 1:
> undefined_fun2();
> break;
> case 4:
> defined_fun();
> break;
> case 8:
> undefined_fun3();
> default:
> undefined_fun();
> }
> return 0;
> }
>
> gets a link time error from gcc.
>
> Dead code elimination needs to be done for all cases where the condition
> value is known at compile time.
>
> Murthy
>
> *From:* Shin-Ming Liu [mailto:shinm...@gmail.com <shinm...@gmail.com>]
> *Sent:* Friday, March 16, 2012 8:57 AM
> *To:* Gang Yu
> *Cc:* open64-devel
> *Subject:* Re: [Open64-devel] Review request for fix the O0-DCE
> bug(bug798)[CG]
>
> Hi Gang,
>
> Your patch proposal is valuable in uncovering some open64 architectural
> decision made in the early design phase. With that in mind, I try to share
> some of my point of view. Others are welcome to chip in.
>
> There is a goal is to reduce the dependency between cg.so and the rest of
> the backend. As the result, cg.so is dependent on be.so and associated
> target specific sharable libraries. The following history might be helpful
> to understand how the original open64 design team follows this principle
> with discipline.
>
> The alias analysis package is part of the wopt.so initially. Once the
> decision was made to export the alias query functionality to cg, the
> designer of the alias package decided to move its functionality to be.so.
>
> In your case, you want to reuse the CFG functionality in wopt in cg.
> Let's set aside all other arguments and only focus on this specific goal, a
> better approach is to turn this CFG functionality into a utility package
> and move it to be.so. You might follow up with an arguement that the cfg
> package in wopt is tightly coupled with everything else in wopt and hence
> this approach is too costly. In that case, I am going to suggest not using
> the CFG package in wopt and seek other alternatives.
>
> Going into one level deeper for the reason why we try to reduce the
> dependency, there is another design goal to enable quick bug triaging with
> a potential to triage bug with script level automation. With the
> backend broken down to multiple sharable libraries, unless there is WHIRL
> definition change, I could use the cg.so from a stable version of compiler
> to mix with the wopt.so that I am currently heavily morphing. This way, I
> could always obtain a very reliable cg for my development to the extend I
> don't even need to rebuild the cg. To some extend, we could even release
> wopt.so to a customer for a minor fix. Of course, Fred would state that we
> will never need to do that because wopt is so stable that we don't need to
> ship a patch release :-)
>
> I appreciate that you have read the email this far. To summarize, I would
> humbly suggest you to take back this patch proposal and re-engineer the fix
> for this bug report since I am not a gatekeeper officially.
>
> Best regards,
>
> Shin
>
> On Fri, Mar 16, 2012 at 5:38 AM, Gang Yu
> <*yugang...@gmail.com*<yugang...@gmail.com>>
> wrote:
> Thanks for the comment.
> On Wed, Mar 14, 2012 at 3:32 PM, Jian-Xin Lai
> <*laij...@gmail.com*<laij...@gmail.com>>
> wrote:
> Some comments:
> 1. Link wopt.so into cg.so is bad. It breaks the modularization. Also,
> the start time is increased at O0/O1 because wopt.so is loaded
> unconditionally.
> The essential idea of this patch is to let OPT_CFG, i.e, WOPT component do
> things in L WHIRL, we want maximum reuse the production modules rather
> than re-invent the wheel. So, CG phase now depends WOPT. Make cg.so depends
> on wopt.so is a preferred simply way.
>
> *). You can't shift the functionality to be.so, that will make be.so
> depend on wopt.so, which then cause other backend executables such as
> lw_inline link wopt.so, this will relate those in essense
> un-related components
> *). dependences between so files are common in current infrastructure, for
> example, lno.so depends on wopt.so and ipl.so
> *). In a long term, we are moving toward staticly linked executables as
> the static library patch checked in and the other similar compiler has
> already done this.
>
> 2. It's not good to add this phase to CG since it's target
> independent. Maybe you can try to add a new phase to be driver.
> This is a target independent phase, so we put it in the
> CG_Generate_Code, the mainline procedure for CG and inevitable path for all
> targets. You can't figure out a target that don't use this function.
>
> 3. For case of switch, it's not good to match the patterns to find the
> candidates of constant. Maybe you can simplify the high level SCF in
> VHO and split this work into 3 parts:
> Compared to the proposed patch, I can only consider this is a suggestion,
> not concrete things.
> part1: simplify the high level SCF in VHO
> I do not see something specially useful at this phase, the dump at VHO
> phase below:
>
> PRAGMA 0 119 <null-st> 0 (0x0) # PREAMBLE_END {line: 1/7}
>
> U8INTCONST 4 (0x4)
> U8STID 0 <2,2,_temp__switch_index0> T<9,.predef_U8,8> {line: 1/10}
> SWITCH 3 1538 {line: 1/21}
>
> U8U8LDID 0 <2,2,_temp__switch_index0> T<9,.predef_U8,8>
> BLOCK {line: 0/0}
> CASEGOTO L258 1 {line: 0/0}
> CASEGOTO L770 4 {line: 0/0}
> CASEGOTO L1026 8 {line: 0/0}
> END_BLOCK
> GOTO L1282 {line: 0/0}
> END_SWITCH
> LABEL L258 0 {line: 1/10}
> VCALL 126 <1,52,undefined_fun2> # flags 0x7e {line: 1/13}
> GOTO L514 {line: 1/14}
> LABEL L770 0 {line: 1/14}
> VCALL 126 <1,50,defined_fun> # flags 0x7e {line: 1/16}
> GOTO L514 {line: 1/17}
> LABEL L1026 0 {line: 1/17}
> VCALL 126 <1,53,undefined_fun3> # flags 0x7e {line: 1/19}
> LABEL L1282 0 {line: 1/19}
> VCALL 126 <1,54,undefined_fun> # flags 0x7e {line: 1/21}
> LABEL L1538 0 {line: 0/0}
> LABEL L514 0 {line: 1/21}
> I4INTCONST 0 (0x0)
> I4RETURN_VAL {line: 1/23}
> part2: simplify the low level BR instructions in the new phase
> part3: build the CFG and remove unreachable code.
>
> The suggested way obviously make things complexed.
> *). You still need CFG
> *). *remove unreachable code* which is not an intended optimisation at O0
> phase.
>
> Regards
> Gang
> 2012/3/7 Gang Yu <*yugang...@gmail.com* <yugang...@gmail.com>>:
> > Hi, list:
> >
> > We have a new design and implementation for fix the gcc incompatible
> > issue, tracked as bug798,bug588,..., i,e. open64 unable to delete the
> > unreachable code under O0 as gcc does.
> >
> > A typical case below:
> >
> > int defined_fun()
> > {
> > }
> > int main()
> > {
> > int t;
> > switch(sizeof(t))
> > {
> > case 1:
> > undefined_fun2();
> > break;
> > case 4:
> > defined_fun();
> > break;
> > case 8:
> > undefined_fun3();
> > default:
> > undefined_fun();
> > }
> > return 0;
> > }
> > Previously, we have proposed invoking the wopt by a special preopt phase
> "q"
> > which includes only copy-prop/DCE phases, however, this approach is not
> > accepted due to
> >
> > *) it obviously increase the compile time due to the essetial,
> compile-time
> > expensive components in WOPT. This is not acceptable by O0 users who are
> > quite sensitive to the compile-time.
> > *) Invoking wopt may cause some stmts losing the dwalf debug info, this
> is
> > not acceptable by people intent to debug the program using "-O0 -g".
> >
> > After that, we have a detailed analysis on the O0 DCE problem, roughly,
> > there are two Unreachable Code Elimination(UCE) cases to handle under O0
> > 1) the if -then-else case:
> > if (sizeof(type x) == A) {
> > ...;
> > } else if (sizeof(type y) == B) {
> > ...;
> > } else {
> > ...
> > }
> > 2). the switch case : just like the above shown case.
> >
> > under O0, open64 can handle the case 1, this is because (sizeof(type x)
> ==
> > A) can be whirl expressed and folded to
> >
> > INTCONST 0
> > FALSEBR/TRUEBE
> >
> > so, unreachable branches then can be indirectly deleted by branch
> handling
> > the case V_BR_NONE or V_BR_ALWAYS
> >
> > All we wanted is to handle the "switch-case", The new suggested way is to
> > build CFG at the beginning of CG_Generate_Code, we do *special const
> > propagation* and then *condition fold* to make the branch as
> >
> > U8INTCONST 4 (0x4)
> > U8STID 0 <2,2,_temp__switch_index0> T<9,.predef_U8,8> {line: 1/10}
> > U8U8LDID 0 <2,2,_temp__switch_index0> T<9,.predef_U8,8>
> > U8INTCONST 4 (0x4)
> > I4U8EQ
> > U8INTCONST 4 be copy propagated to:
> > U8U8LDID 0 <2,2,_temp__switch_index0> T<9,.predef_U8,8>
> >
> > Then
> > U8INTCONST 4 (0x4)
> > U8INTCONST 4 (0x4)
> > I4U8EQ
> > folded to
> > INTCONST 1
> > TRUEBR
> >
> > then let the branch expansion routine do things.
> >
> > We have some basic design rules:
> > *) make things as simple as possible, the introduced procedure is only
> for
> > gnu compatiblity and no intent to be optimisation. So, we only do limited
> > const prop and expression fold, no real DCE or other changes to CFG.
> > *) maxium leverage the current production modules, do *not* invent new
> > things
> > a). invoke the opt_cfg for CFG building
> > b). port the OPT_RVI_EMIT for cfg emit and region handling.
> >
> > Attached is the patch, self checking the patch, we do not find the
> issues
> > by preopt "q" phase, since,
> > *) the only compile time expense is to build-up the CFG, then emit back,
> > *) we only modify kids of (FALSE/TRUE) branch stmts, do not corrupt the
> > dwalf info.
> >
> > Applying the patch, we build up the linux kernel using "-O0 -g".
> >
> > Would a gatekeeper please help review the patch? Thanks a lot.
> >
> >
> > Regards
> > Gang
> >
> >
> ------------------------------------------------------------------------------
> > Virtualization & Cloud Management Using Capacity Planning
> > Cloud computing makes use of virtualization - but cloud computing
> > also focuses on allowing computing to be delivered as a service.
> > *http://www.accelacomm.com/jaw/sfnl/114/51521223/*<http://www.accelacomm.com/jaw/sfnl/114/51521223/>
> > _______________________________________________
> > Open64-devel mailing list
> > *Open64-devel@lists.sourceforge.net*<Open64-devel@lists.sourceforge.net>
> > *https://lists.sourceforge.net/lists/listinfo/open64-devel*<https://lists.sourceforge.net/lists/listinfo/open64-devel>
> >
>
>
>
> --
> Regards,
> Lai Jian-Xin
>
>
>
> ------------------------------------------------------------------------------
> This SF email is sponsosred by:
> Try Windows Azure free for 90 days Click Here
> *http://p.sf.net/sfu/sfd2d-msazure* <http://p.sf.net/sfu/sfd2d-msazure>
> _______________________________________________
> Open64-devel mailing list
> *Open64-devel@lists.sourceforge.net* <Open64-devel@lists.sourceforge.net>
> *https://lists.sourceforge.net/lists/listinfo/open64-devel*<https://lists.sourceforge.net/lists/listinfo/open64-devel>
>
>
>
------------------------------------------------------------------------------
This SF email is sponsosred by:
Try Windows Azure free for 90 days Click Here
http://p.sf.net/sfu/sfd2d-msazure
_______________________________________________
Open64-devel mailing list
Open64-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/open64-devel
