RVM

Handle instanceof and checkcast in ShortArray scalar replacer

Details

  • Type: Improvement Improvement
  • Status: Closed Closed
  • Priority: Minor Minor
  • Resolution: Fixed
  • Affects Version/s: None
  • Fix Version/s: 3.0
  • Component/s: Compiler: Optimizing
  • Labels:
    None
  • Number of attachments :
    0

Description

In playing with inlining heuristics, I discovered some missing unsupprted operations clauses in ShortArrayReplacer. We don't handle checkcast operations, but they also aren't in the unsupported list. As a quick fix, I'm adding them as unsupported operations in the containsUnsupportedUse method of this class (running pre-commit now).

However, it would be better to actually support them. If we enough information to scalar replace the array, it really seems like we should be able to evaluate instanceof and checkcast operations at compile time.

Witrh my tweak to the inlining heuristics, until we handle checkcast in ShortArrayReplacer we're missing a newly revealed opportunity to do short array scalar replacement in the java/security.VMAccessController. getContext. method.

Activity

Ascending order - Click to sort in descending order
Hide
Permalink
Ian Rogers added a comment -

I've got support for these in a world of mine, I could get the changes in pre-2.9.3.

Show
Ian Rogers added a comment - I've got support for these in a world of mine, I could get the changes in pre-2.9.3.
Hide
Permalink
Ian Rogers added a comment -

To be more precise, the problem that I see is to do with creating a default operand for the array (which is null), then getting the register type from this operand - so the null type reference. When we iterate and try to simplify operations, we can't remove the redundant casts. instanceof is more problematic as "null instanceof foo" is false. See RVM-448 (track non-nullness in register operands).

Show
Ian Rogers added a comment - To be more precise, the problem that I see is to do with creating a default operand for the array (which is null), then getting the register type from this operand - so the null type reference. When we iterate and try to simplify operations, we can't remove the redundant casts. instanceof is more problematic as "null instanceof foo" is false. See RVM-448 (track non-nullness in register operands).
Hide
Permalink
Ian Rogers added a comment -

Patch committed in r14147, Dave can you see if this clears up your issue?

Show
Ian Rogers added a comment - Patch committed in r14147, Dave can you see if this clears up your issue?
Hide
Permalink
Ian Rogers added a comment -

Using the OTH to look at java.security.VMAccessController.getContext there are 2 array allocations, one escapes and the other is of an unknown length. I imagine the case is exposed when this code is inlined, is there an example of this?

Show
Ian Rogers added a comment - Using the OTH to look at java.security.VMAccessController.getContext there are 2 array allocations, one escapes and the other is of an unknown length. I imagine the case is exposed when this code is inlined, is there an example of this?
Hide
Permalink
David Grove added a comment -

yes, it shows up in the bootimage. Somewhere in the class library code relatating to stack crawling to check access permissions before doing something.

Show
David Grove added a comment - yes, it shows up in the bootimage. Somewhere in the class library code relatating to stack crawling to check access permissions before doing something.
Hide
Permalink
Ian Rogers added a comment -

I believe the clean up of the IR performed by r14147 fixed this particular issue. However, r14640 now adds support for instanceof and checkcast elimination by both aggregate replacement compiler phases. Adding debug information didn't show any cases of this being necessary in the boot image or when running eclipse.

Show
Ian Rogers added a comment - I believe the clean up of the IR performed by r14147 fixed this particular issue. However, r14640 now adds support for instanceof and checkcast elimination by both aggregate replacement compiler phases. Adding debug information didn't show any cases of this being necessary in the boot image or when running eclipse.
Hide
Permalink
David Grove added a comment -

reopening so I can modify fix target to 3.0

Show
David Grove added a comment - reopening so I can modify fix target to 3.0

People

  • Assignee:
    Ian Rogers
    Reporter:
    David Grove
Vote (0)
Watch (0)

Dates

  • Created:
    Updated:
    Resolved: