Re: [Open64-devel] Review request for fix the O0-DCE bug(bug798)[CG]
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
[X]
From: Shin-Ming Liu [mailto: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.commailto:yugang...@gmail.com wrote:
Thanks for the comment.
On Wed, Mar 14, 2012 at 3:32 PM, Jian-Xin Lai
laij...@gmail.commailto: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 T9,.predef_U8,8 {line: 1/10}
SWITCH 3 1538
Re: [Open64-devel] Review request for fix the O0-DCE bug(bug798)[CG]
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 T4,.predef_I4,4 {line: 1/10}
I4I4LDID 0 2,2,size T4,.predef_I4,4
I4STID 0 2,3,_temp__switch_index0 T4,.predef_I4,4 {line: 1/11}
I4I4LDID 0 2,3,_temp__switch_index0 T4,.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.comwrote:
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
Re: [Open64-devel] Fix to bug 917
which part of const_fold functions do you intend to use? Or are you writing your own? Note also that you will need to observe IEEE settings for floats Sun On Fri, Mar 16, 2012 at 3:55 AM, Gilmore, Doug doug.gilm...@amd.com wrote: Other Fortran front ends are capable of folding calls to intrinsics that have constant arguments in parameter statements, but this functionality is missing in the Open64 front end. As a first cut, I went to the process of handling this for the real intrinsic. The patch and test case is attached. We will be adding more of this functionality for other intrinsics in the near future. Can a gatekeeper review this change for me? Thanks, Doug -- 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 -- 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
Re: [Open64-devel] Review request for fix the O0-DCE bug(bug798)[CG]
I originally sent a message on this, noting the issue in the front end.
I'll forward the message again.
Doug
From: Chandrasekhar Murthy [mailto:mur...@sgi.com]
Sent: Saturday, March 17, 2012 8:56 AM
To: Gang Yu
Cc: open64-devel
Subject: Re: [Open64-devel] Review request for fix the O0-DCE bug(bug798)[CG]
Hi Gang,
I don't build or access Open64 sources.
What is the WHIRL coming out from the frontend for the switch value?
If it is not INTCONST, then the frontend needs to be fixed to emit it.
Once that is done, you can modify VHO to get rid of the dead code.
Murthy
From: Gang Yu [mailto:yugang...@gmail.com]
Sent: Saturday, March 17, 2012 8:07 AM
To: Chandrasekhar Murthy
Cc: Shin-Ming Liu; open64-devel
Subject: Re: [Open64-devel] Review request for fix the O0-DCE bug(bug798)[CG]
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 T4,.predef_I4,4 {line: 1/10}
I4I4LDID 0 2,2,size T4,.predef_I4,4
I4STID 0 2,3,_temp__switch_index0 T4,.predef_I4,4 {line: 1/11}
I4I4LDID 0 2,3,_temp__switch_index0 T4,.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.commailto: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
[%20]
From: Shin-Ming Liu [mailto: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.commailto:yugang...@gmail.com wrote:
Thanks for the comment.
On Wed, Mar 14, 2012 at 3:32 PM, Jian-Xin Lai
laij...@gmail.commailto: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
Re: [Open64-devel] Fix to bug 917
-Original Message- From: Sun Chan [mailto:sun.c...@gmail.com] Sent: Saturday, March 17, 2012 10:37 AM To: Gilmore, Doug Cc: open64-devel Subject: Re: [Open64-devel] Fix to bug 917 which part of const_fold functions do you intend to use? Or are you writing your own? I am using the same mechanism as the other folding operations do. Folding is done in a routine written in Fortran. Note also that you will need to observe IEEE settings for floats Sun Given that we use the same execution mechanism that used for the folding of other operations. If it is being done incorrectly in this patch, it be incorrect for all folding operations. Have we seen other constant foldings incorrectly folded? Doug On Fri, Mar 16, 2012 at 3:55 AM, Gilmore, Doug doug.gilm...@amd.com wrote: Other Fortran front ends are capable of folding calls to intrinsics that have constant arguments in parameter statements, but this functionality is missing in the Open64 front end. As a first cut, I went to the process of handling this for the real intrinsic. The patch and test case is attached. We will be adding more of this functionality for other intrinsics in the near future. Can a gatekeeper review this change for me? Thanks, Doug - - 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 -- 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
Re: [Open64-devel] Fix to bug 917
The BE uses the same const fold in simplifier. I think there is a way to include simplifier in fortran FE. My question is, are you using that? Or writing your own? If you are writing your own, I just want to point out the potential problems in terms of rounding/... IEEE issue Sun On Sun, Mar 18, 2012 at 4:08 AM, Gilmore, Doug doug.gilm...@amd.com wrote: -Original Message- From: Sun Chan [mailto:sun.c...@gmail.com] Sent: Saturday, March 17, 2012 10:37 AM To: Gilmore, Doug Cc: open64-devel Subject: Re: [Open64-devel] Fix to bug 917 which part of const_fold functions do you intend to use? Or are you writing your own? I am using the same mechanism as the other folding operations do. Folding is done in a routine written in Fortran. Note also that you will need to observe IEEE settings for floats Sun Given that we use the same execution mechanism that used for the folding of other operations. If it is being done incorrectly in this patch, it be incorrect for all folding operations. Have we seen other constant foldings incorrectly folded? Doug On Fri, Mar 16, 2012 at 3:55 AM, Gilmore, Doug doug.gilm...@amd.com wrote: Other Fortran front ends are capable of folding calls to intrinsics that have constant arguments in parameter statements, but this functionality is missing in the Open64 front end. As a first cut, I went to the process of handling this for the real intrinsic. The patch and test case is attached. We will be adding more of this functionality for other intrinsics in the near future. Can a gatekeeper review this change for me? Thanks, Doug - - 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 -- 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
Re: [Open64-devel] Review request for fix the O0-DCE bug(bug798)[CG]
with Doug's forwarded message, I recall the previous conversation.
If we can fix VHO, that should be fine.
The other approach is to distinguish between -O0 vs -O0 -g where the later
will not turn on any optimization, inline ... whatsoever. -O0 really means
-O0 -P:=on, which will do some simple optimization.
Or we simply add a -Linux_kernel option that will do -O0 -P ...
Just a thought
Sun
On Sun, Mar 18, 2012 at 4:00 AM, Gilmore, Doug doug.gilm...@amd.com wrote:
I originally sent a message on this, noting the issue in the front end.**
**
** **
I’ll forward the message again.
** **
Doug
** **
*From:* Chandrasekhar Murthy [mailto:mur...@sgi.com]
*Sent:* Saturday, March 17, 2012 8:56 AM
*To:* Gang Yu
*Cc:* open64-devel
*Subject:* Re: [Open64-devel] Review request for fix the O0-DCE
bug(bug798)[CG]
** **
Hi Gang,
** **
I don’t build or access Open64 sources.
What is the WHIRL coming out from the frontend for the switch value?
If it is not INTCONST, then the frontend needs to be fixed to emit it.
Once that is done, you can modify VHO to get rid of the dead code.
** **
Murthy
** **
--
*From:* Gang Yu [mailto:yugang...@gmail.com]
*Sent:* Saturday, March 17, 2012 8:07 AM
*To:* Chandrasekhar Murthy
*Cc:* Shin-Ming Liu; open64-devel
*Subject:* Re: [Open64-devel] Review request for fix the O0-DCE
bug(bug798)[CG]
** **
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 T4,.predef_I4,4 {line: 1/10}
I4I4LDID 0 2,2,size T4,.predef_I4,4
I4STID 0 2,3,_temp__switch_index0 T4,.predef_I4,4 {line: 1/11}
I4I4LDID 0 2,3,_temp__switch_index0 T4,.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
