I'm ok with the fix
Sun
On Sat, Jul 2, 2011 at 1:54 AM, Mathew, Pallavi <pallavi.mat...@amd.com> wrote:
> Hi,
>
> Can a gatekeeper please review the attached patch that fixes an issue with
> dead scalar references?
>
>
>
> Testcase:
>
> extern double sqrt(double);
>
> int global_arr[4];
>
> void SetPoints()
>
> {
>
> int i, r0;
>
> for (i=0; i<10; i++) {
>
> r0 = global_arr[3];
>
> sqrt(r0 * r0);
>
> }
>
> }
>
>
>
> Compiling with 'opencc -O3 -apo' gives:
>
> ### Assertion failure at line 710 of
> .../../../osprey/be/lno/access_vector.h:
>
> ### Compiler Error in file sample.c during Loop Nest Optimizer phase:
>
> ### SYMBOL::Init(WN*) called with opcode 66612
>
> opencc INTERNAL ERROR: .../x86_64-open64-linux/4.2/be returned non-zero
> status 1
>
>
>
> Problem/Fix Description:
>
> The error is caused due to a dangling pointer leftover from an incomplete
>
> cleanup after array substitution removes a whirl node. The error shows up
>
> only for calls to builtin math function 'sqrt' (not sure if there are more
>
> such functions) where the original call is replaced by a builtin and a new
>
> call is placed depending on the accuracy of the result. The problem will not
>
> show with '-fno-math-errno'.
>
> The fix adds a cleanup function to remove scalar references to WHIRL nodes
>
> marked for deletion.
>
>
>
> The loop in sample.c:
>
> for (i=0; i<10; i++) {
>
> r0 = global_arr[3]; //L1
>
> sqrt(r0 * r0); //L2
>
> }
>
>
>
> The CALL_INFO corresponding to L2 has 'r0' as a scalar_use.
>
> Array substitution replaces 'r0' with 'global_arr[3] and deletes whirl node
> for 'r0'.
>
> But the reference to 'r0' still exists within call_info of 'sqrt', which
>
> results in an inconsistency (assertion failure) in array_loop_info that is
> detected
>
> when merging a call's scalar uses as part auto parallelization.
>
>
>
> Note that had the call to 'sqrt' been a VCALL it would not go through array
> substitution.
>
> 'sqrt' being a builtin, the VCALL is replaced (in the absence of
> -fno-math-errno) by
>
> F8SQRT, result saved in temp_var and depending on whether or not the result
> is a NAN,
>
> a call is placed to F8CALL_SQRT. (See trace file with -Wb,-tra).
>
> -apo is needed to expose error because Annotations of Call
>
> (NSE_Annotate_Scalar_Call called by IPA_LNO_Map_Calls) only occurs under
> autopar.
>
>
>
> The patch to function 'DeleteElement' in osprey/be/com/cxx_template.h is to
> avoid
>
> a compile error (during compiler build) caused by absence of a suitable
> assignment
>
> operator for SCALAR_NODE and SCALAR_REF. This is issue was not exposed
> earlier because
>
> existing calls to 'DeleteTop' were only on stacks of pointers. An alternate
> patch would
>
> be to define dummy assignment operators like the one shown below, although I
> prefer the
>
> patch shown in the attachment.
>
>
>
> Index: osprey/be/com/dep_graph.h
>
> ===================================================================
>
> --- osprey/be/com/dep_graph.h (revision 1418)
>
> +++ osprey/be/com/dep_graph.h (working copy)
>
> @@ -815,6 +815,11 @@
>
> Statement_Number = scalar_ref.Statement_Number;
>
> return *this;
>
> }
>
> + SCALAR_REF& operator=(void*) {
>
> + Wn =NULL;
>
> + Statement_Number = 0;
>
> + return *this;
>
> + }
>
> };
>
>
>
> // all the references to a particular SYMBOL
>
> @@ -830,6 +835,10 @@
>
> _scalar_ref_stack = CXX_NEW(SCALAR_REF_STACK(pool),pool);
>
> _scalar = scalar;
>
> }
>
> + SCALAR_NODE& operator=(void*) {
>
> + _scalar_ref_stack = NULL;
>
> + return *this;
>
> + }
>
> INT Elements() const { return _scalar_ref_stack->Elements(); };
>
> SCALAR_REF *Bottom_nth(INT i) { return &_scalar_ref_stack->Bottom_nth(i);
> };
>
> SCALAR_REF *Top_nth(INT i) { return &_scalar_ref_stack->Top_nth(i); };
>
>
>
> Thanks.
>
> Pallavi
>
> ------------------------------------------------------------------------------
> All of the data generated in your IT infrastructure is seriously valuable.
> Why? It contains a definitive record of 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-d2d-c2
> _______________________________________________
> Open64-devel mailing list
> Open64-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/open64-devel
>
>
------------------------------------------------------------------------------
All of the data generated in your IT infrastructure is seriously valuable.
Why? It contains a definitive record of 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-d2d-c2
_______________________________________________
Open64-devel mailing list
Open64-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/open64-devel
