RFS(M): 8005031: Some cleanup in c2 to prepare for incremental inlining support

[Roland Westrelin]

Hi Vladimir,

Thanks for reviewing this.


> Why you removed asserts in callnode.cpp?

If inlining happens after the graph has been transformed then useless projections may have gone away. Anyway, this doesn't belong in this cleanup so I moved it to the incremental inlining change. I put the assert back but it's disabled if inlining is happening after the first round of parsing.

> I think "chunk" word is not appropriate for this. Maybe "Event" or something:
> 
> PrintInliningChunk --> PrintInliningEvent
> _print_inlining_chunks --> _print_inlining_list
> 
> Allocate the list in comp arena for use in later inlining: new(comp_arena())

Ok.

> LogCompilation output will be also affected. I remember fixing the tool for StringOpts. Could you verify that it works and fix it if it does not?

It's indeed broken but it doesn't look like a trivial fix to me. Can this be done separately? If yes, I'll file a new CR.
Also: in the current LogCompilation late_inline entries are listed at the end:

13    spec.benchmarks._213_javac.ScannerInputStream::read (513 bytes)
    @ 32 java.io.InputStream::read (0 bytes)
      type profile java/io/InputStream -> java/io/BufferedInputStream (100%)
    @ 32 java.io.BufferedInputStream::read (49 bytes)
    @ 95 java.io.InputStream::read (0 bytes)
      type profile java/io/InputStream -> java/io/BufferedInputStream (100%)
    @ 95 java.io.BufferedInputStream::read too big
    @ 151 java.io.InputStream::read (0 bytes)
    @ 151 java.io.InputStream::read (0 bytes)
    @ 408 spec.benchmarks._213_javac.Environment::error never executed
    @ 435 java.io.InputStream::read (0 bytes)
    @ 469 java.io.InputStream::read (0 bytes)
late inline:
    @ 32 java.io.BufferedInputStream::read (49 bytes) (end time: 1.7170 nodes: 792 live: 538)
      @ 12 java.io.BufferedInputStream::fill too big
      @ 29 java.io.BufferedInputStream::getBufIfOpen (21 bytes)
    @ 32 java.io.BufferedInputStream::read (49 bytes)
      @ 29 java.io.BufferedInputStream::getBufIfOpen (21 bytes) (end time: 1.7180 nodes: 826 live: 569)

(strange that @29 is twice in the late inline section)
But if there's a lot of late inlining going on, wouldn't we want to have everything together:
13    spec.benchmarks._213_javac.ScannerInputStream::read (513 bytes)
    @ 32 java.io.InputStream::read (0 bytes)
      type profile java/io/InputStream -> java/io/BufferedInputStream (100%)
    @ 32 java.io.BufferedInputStream::read (49 bytes) (end time: 1.7170 nodes: 792 live: 538)
      @ 12 java.io.BufferedInputStream::fill too big
      @ 29 java.io.BufferedInputStream::getBufIfOpen (21 bytes)
    @ 95 java.io.InputStream::read (0 bytes)
      type profile java/io/InputStream -> java/io/BufferedInputStream (100%)
    @ 95 java.io.BufferedInputStream::read too big
    @ 151 java.io.InputStream::read (0 bytes)
    @ 151 java.io.InputStream::read (0 bytes)
    @ 408 spec.benchmarks._213_javac.Environment::error never executed
    @ 435 java.io.InputStream::read (0 bytes)
    @ 469 java.io.InputStream::read (0 bytes)

with maybe an annotation that marked some call site as late inline. What do you think?

> I think you should put it on igvn._worklist because PhaseIterGVN::optimize() does removal of nodes with outcount() == 0. I don't think it is safe to do it inside Ideal() call.

Ok.

> Also, file RFE (starter_task) to find all similar cases (may be add the check for input nodes in transform_old()).

Ok.

Here is a new webrev:

http://cr.openjdk.java.net/~roland/8005031/webrev.01/

Roland.