RFR(S): 8050022: linux-sparcv9: assert(SharedSkipVerify || obj->is_oop()) failed: sanity check

[John Rose]

On Sep 10, 2014, at 8:16 AM, David Chase <david.r.chase at oracle.com> wrote:

> Perhaps it would be better to say (after the comment)
>  int float_index = 1 + (j << 1);
> and repeat float_index three times,
> and likewise for the even index in the double case
>  int double_index = (j<<1)

I agree with that point.

> Other than that, I like it, especially the typos/spellos that you caught in associated files.

+1

Style comment:  The name "_sp_adjustment_floating_point" should be "_floating_point_sp_adjustment" to match HotSpot's local pattern for C++ accessors.  Also, since it is a boolean, the accessor should sound like a boolean (not an "adjustment"), like "adjust_sp_for_floating_point".

But, I have a bigger issue which make make that moot:  The "adjust_sp_for_floating_point" dance is awkward but acceptable (maybe with amendments) if it is truly a system-specific issue.  I don't think it is system-specific, but ABI-specific.  What does the ABI documentation say about this case?

I googled to http://www.sparc.org/technical-documents-test-2/specifications/ and then to http://www.sparc.org/wp-content/uploads/2014/01/SCD.2.4.1.pdf.gz  .

That document is (or should be) normative for all implementations of C on sparc, including both Linux and Solaris.  On page 3P-12, under "Floating Arguments", it says:

> When a callee prototype exists, and does not indicate variable arguments, floating- point values assigned to locations %sp+BIAS+128 through %sp+BIAS+248 will be promoted into floating-point registers, as shown above.

By "promoted" it means that the argument is located in two places, an unused spill slot in the "parameter array" (starts at %sp+BIAS+128), and a live float register.  In most cases, there are 6 or fewer arguments of any type, and the standard parameter array slots (%sp+BIAS+128 to %sp+BIAS+176 exclusive) serve as shadow slots.  But floating point registers %d6 to %d16 require slots beyond that (up to %sp+BIAS+248, as the spec. says).

This means that the whole adjust_sp_for_floating_point dance should go away, and the parameter array extension needs to be done on all LP64 sparc systems.

Sparc does it this way in order to have a very simple varargs implementation (one contiguous parameter array, allocated by the caller).  If there are other systems which have a similar C ABI, we will have a similar bug for those also.  Would you mind looking at the corresponding code for other cpus we support, and seeing if there is potentially the same bug there also?  If so we can check the relevant ABI docs and perhaps file bugs for those guys also.

Thanks!

— John
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/attachments/20140910/c8f21580/attachment-0001.html>