Hi Sun,
Thanks for the comments. I will further look into this and get back.
The bug fix 14470 has two parts. If the expression is found constant, the
expression is replaced with constant 1. If the expression is not found to be a
constant expression, it is replaced with a constant ZERO. While I agree with
the simplification of constant expressions (with CONSTANT 1), I am not sure,
if the 2nd simplification (non-constant expression) will be guaranteed not to
remove any useful expressions/definitions. I have removed the 2nd part in the
fix.
Regards,
Shivaram
From: Sun Chan [mailto:sun.c...@gmail.com]
Sent: Sunday, September 18, 2011 4:01 AM
To: Rao, Shivarama
Cc: open64-devel@lists.sourceforge.net
Subject: Re: [Open64-devel] Code Review request for bug787 [WOPT]
I don't fully understand why you remove the code in simplifier. Your rationale
doesn't seem logical. On top of that, the comment in the "removed" code
indicates a certain bug, meanging, if removed, that bug will come back. There
is nothing wrong with replacing a const bool expression with 0 or 1. I doubt
you are really fixing the problem with this fix
Sun
On Fri, Sep 16, 2011 at 6:11 AM, Rao, Shivarama
<shivarama....@amd.com<mailto:shivarama....@amd.com>> wrote:
Hi,
Could a gatekeeper please review the fix done for bug 787?
The problem is with the simplification of INTRN_CONSTANT_P intrinsic
(buitin_constant_p operand). The SIMPNODE_SimplifyIntrinsic function replaces
the INTRN_CONSTANT_P intrinsic with a constant operand 1 when found that all
operands are constants and with a 0 when they are not constants. When replacing
the intrinsic function, the current results are deleted.
In the current test case, the current result happen to be the input for
ASM_STMT (inlined assembly statements). SPRE_rename_var throws assertion for
zero_version_cr for ASM_STMT operands since their definitions are deleted.
Since the zeroing of INTRN_CONSTANT_P is done in lower_expr function while
lowering to CG, I feel it is better to remove the zeroing at
SIMPNODE_SimplifyIntrinsic function and the change is done that way.
The patch file and a simplified testcase is attached. The assertion can be
reproduced with the debug build compiler.
Regards,
Shivaram
------------------------------------------------------------------------------
BlackBerry® DevCon Americas, Oct. 18-20, San Francisco, CA
http://p.sf.net/sfu/rim-devcon-copy2
_______________________________________________
Open64-devel mailing list
Open64-devel@lists.sourceforge.net<mailto:Open64-devel@lists.sourceforge.net>
https://lists.sourceforge.net/lists/listinfo/open64-devel
------------------------------------------------------------------------------
BlackBerry® DevCon Americas, Oct. 18-20, San Francisco, CA
http://p.sf.net/sfu/rim-devcon-copy2
_______________________________________________
Open64-devel mailing list
Open64-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/open64-devel
