RFR(XS): Provide information when hitting a HaltNode for architectures other than x86

[Doerr, Martin]

Hi lx,

> If I delete size(4) in ppc.ad,  hotspot can work out the correct size of
> instruction chunk, can't it?
Yes.

> In reality, I observe that a HaltNode always follows the uncommon_trap call.
> Christian also observed that in JDK-8022574.
> Isn't uncommon trap a one-way ticket for all architectures? I feel the control
> flow never returns after uncommon_trap, why do we generate a HaltNode
> after that? Nevertheless, it's a separated issue.
I think the HaltNode insertion should get removed from GraphKit::uncommon_trap before or with your change. Uncommon_traps are often used.

Other usages of HaltNode may be rare enough. (I haven't checked that.)

Best regards,
Martin


> -----Original Message-----
> From: Liu, Xin <xxinliu at amazon.com>
> Sent: Dienstag, 5. Mai 2020 01:27
> To: Doerr, Martin <martin.doerr at sap.com>; hotspot-compiler-
> dev at openjdk.java.net
> Subject: Re: RFR(XS): Provide information when hitting a HaltNode for
> architectures other than x86
> 
> Hi, Martin,
> 
> Thank you to review it and build it on s390 and PPC!
> 
> If I delete size(4) in ppc.ad,  hotspot can work out the correct size of
> instruction chunk, can't it?
> I found most of instructions in ppc.ad have size(xx), but there're a couple of
> exceptions:  cacheWB & cacheWBPreSync.
> 
> I think it should be in product hotspot.  Here are my arguments.
> 1.  Crash reports of release build are more common.
> Some customers even don't bother trying again with a debug build.
> 
> Let me take the crash report on aarch64 as an example. I paste the
> comparison before and after.
> https://bugs.openjdk.java.net/browse/JDK-
> 8230552?focusedCommentId=14334977&page=com.atlassian.jira.plugin.syst
> em.issuetabpanels%3Acomment-tabpanel#comment-14334977
> 
> Without stop(_halt_reason), what we know is "there's a bug in C2-generated
> code". If the method is big,  which is very likely because of inlining, it's easy to
> get lost.
> 
> I feel it's more helpful with the patch. We can locate which HaltNode it is by
> searching _halt_reason in codebase.
> Hopefully, we can find the culprit method from "Compilation events".
> 
> 2. HaltNode is rarely generated and will be removed if it's dead.
> IMHO, the semantic of that Node is "halt". If It remains after optimizer or
> lowering to mach-nodes,  something wrong and unrecoverable happened in
> the compilers. After we fix the compiler bug,  it should be gone.  That's is too
> say, it shouldn't cause any problem about code size in ideal cases.
> 
> In reality, I observe that a HaltNode always follows the uncommon_trap call.
> Christian also observed that in JDK-8022574.
> Isn't uncommon trap a one-way ticket for all architectures? I feel the control
> flow never returns after uncommon_trap, why do we generate a HaltNode
> after that? Nevertheless, it's a separated issue.
> 
> Let me make another revision to fix PPC and I found that sparc.ad is gonna
> gone.
> 
> Thanks,
> --lx
> 
> 
> On 5/4/20, 1:47 PM, "Doerr, Martin" <martin.doerr at sap.com> wrote:
> 
>     CAUTION: This email originated from outside of the organization. Do not
> click links or open attachments unless you can confirm the sender and know
> the content is safe.
> 
> 
> 
>     Hi lx,
> 
>     the size attribute is wrong on PPC64 (stop is larger than 4 Bytes). S390
> builds fine.
>     I've only run the build. No tests.
> 
>     Should this feature be debug-only?
>     Do we want the lengthy code emitted in product build?
> 
>     Best regards,
>     Martin
> 
> 
>     > -----Original Message-----
>     > From: hotspot-compiler-dev <hotspot-compiler-dev-
>     > bounces at openjdk.java.net> On Behalf Of Liu, Xin
>     > Sent: Donnerstag, 30. April 2020 06:03
>     > To: hotspot-compiler-dev at openjdk.java.net
>     > Subject: RFR(XS): Provide information when hitting a HaltNode for
>     > architectures other than x86
>     >
>     > Hi,
>     >
>     > Could you review this small patch?  It unifies codegen of HaltNode for
> other
>     > architectures.
>     > JBS: https://bugs.openjdk.java.net/browse/JDK-8230552
>     > Webrev: https://cr.openjdk.java.net/~xliu/8230552/00/webrev/
>     >
>     > I tested on aarch64.  It generates the same crash report as x86_64 when
> it
>     > does hit HaltNode.  Halt reason is displayed. I paste report on the JBS.
>     > I ran hotspot:tier1 on aarch64 fastdebug build.  It passed except for 3
>     > relevant failures[1].
>     >
>     > I plan to do that on aarch64 only, but it’s trivial on other architectures, so I
>     > bravely modified them all.  May I invite s390, SPARC arm32 maintainers
> take a
>     > look at it?
>     > If it goes through the review, I hope a sponsor can help me to push the
>     > submit repo and see if it works.
>     >
>     > [1] those 3 tests failed on aarch64 with/without my changes.
>     > gc/shenandoah/mxbeans/TestChurnNotifications.java#id2
>     > gc/shenandoah/mxbeans/TestChurnNotifications.java#id1
>     > gc/shenandoah/mxbeans/TestPauseNotifications.java#id1
>     >
>     > thanks,
>     > -lx
>     >
>