buggy behavior in AMD64NodeLIRBuilder.emitCompareBranchMemory
Lukas Stadler
lukas.stadler at oracle.com
Thu May 15 14:15:39 UTC 2014
Hi guys,
thanks so much for the detailed bug report, and for narrowing it down further.
It was a bug that happened when inferring stamps within the canonicalizer (which is run after tail duplication).
In rare cases, meeting multiple ObjectStamps to determine the stamp of a phi, with one of the phi inputs being illegal, led to an illegal stamp on the phi.
The correct behavior is to just ignore the illegal type, since this corresponds to an unreachable path.
Comparing something with an illegal stamp to anything led to “false”, since there’s no value that can fulfill “illegal”.
This subsequently removed the isInt check.
I just pushed a fix, should be through later today (look for "(f6264f499455) correctly handle illegal stamps in ObjectStamp.meet”).
- Lukas
On 15 May 2014, at 8:11, Yudi Zheng <yudi.zheng at usi.ch> wrote:
> The simplified test case I sent yesterday is the following:
>
> public class TailDuplicationTest {
>
> interface I {
> }
>
> enum P implements I {
> A, B
> }
>
> final static I CONST_I = new I() {
> };
>
> public static I foo(int i) {
> if (i < 32) {
> return P.A;
> } else if (i < 64) {
> return P.B;
> } else {
> return CONST_I;
> }
> }
>
> public int bar(int i) {
> P instance = (P) foo(i);
>
> if (instance != P.B) {
> return 0;
> }
>
> return 1;
> }
>
> public static void main(String[] args) {
> TailDuplicationTest t = new TailDuplicationTest();
>
> for (int i = 0; i < 100000; i++) {
> foo(i % 128);
> t.bar(i % 64);
> }
> }
>
> }
>
> Look at the graph of TailDuplicationTest.bar, the "return 1" branch is dropped between LoopFullUnroll and TailDuplication.
> It is caused by a stamp evaluation which compares an illegal "a!# -" with "a!# LTailDuplicationTest$P;"
> The illegal "a!# -" is generated by ValuePhiNode.inferPhiStamp…
>
> I inserted graph dumping at TailDuplicationPhase#328 and ObjectEqualsNode#64. Please have a look at the stamp changes
> on "48 ValuePhi()" (hopefully the same ID),
>
> diff -r 55977f9fa56f graal/com.oracle.graal.nodes/src/com/oracle/graal/nodes/calc/ObjectEqualsNode.java
> --- a/graal/com.oracle.graal.nodes/src/com/oracle/graal/nodes/calc/ObjectEqualsNode.java Sun May 11 22:00:06 2014 +0200
> +++ b/graal/com.oracle.graal.nodes/src/com/oracle/graal/nodes/calc/ObjectEqualsNode.java Thu May 15 08:07:39 2014 +0200
> @@ -25,6 +25,7 @@
> import com.oracle.graal.api.meta.*;
> import com.oracle.graal.api.meta.ProfilingInfo.*;
> import com.oracle.graal.compiler.common.calc.*;
> +import com.oracle.graal.debug.*;
> import com.oracle.graal.graph.*;
> import com.oracle.graal.graph.spi.*;
> import com.oracle.graal.nodes.*;
> @@ -62,6 +63,10 @@
> if (GraphUtil.unproxify(forX) == GraphUtil.unproxify(forY)) {
> return TriState.TRUE;
> } else if (forX.stamp().alwaysDistinct(forY.stamp())) {
> + if ("bar".equals(graph().method().getName())) {
> + Debug.dump(graph(), "During canonicalizer phase in tail duplication, ObjectEqualsNode#64");
> + }
> +
> return TriState.FALSE;
> } else {
> return super.evaluate(constantReflection, forX, forY);
> @@ -119,7 +124,7 @@
> /*
> * One of the two objects has identity, the other doesn't. In code, this looks like
> * "Integer.valueOf(a) == new Integer(b)", which is always false.
> - *
> + *
> * In other words: an object created via valueOf can never be equal to one created
> * by new in the same compilation unit.
> */
> diff -r 55977f9fa56f graal/com.oracle.graal.phases.common/src/com/oracle/graal/phases/common/TailDuplicationPhase.java
> --- a/graal/com.oracle.graal.phases.common/src/com/oracle/graal/phases/common/TailDuplicationPhase.java Sun May 11 22:00:06 2014 +0200
> +++ b/graal/com.oracle.graal.phases.common/src/com/oracle/graal/phases/common/TailDuplicationPhase.java Thu May 15 08:07:39 2014 +0200
> @@ -326,6 +326,9 @@
> phi.setMerge(mergeAfter);
> }
> }
> + if ("bar".equals(graph.method().getName())) {
> + Debug.dump(graph, "Before canonicalizer phase in tail duplication, TailDuplicationPhase#328");
> + }
> canonicalizer.applyIncremental(graph, phaseContext, startMark);
> Debug.dump(graph, "After tail duplication at %s", merge);
> }
>
>
> - Yudi
>
> On 15 May 2014, at 06:12, Tom Rodriguez <tom.rodriguez at oracle.com<mailto:tom.rodriguez at oracle.com>> wrote:
>
> I think this is actually a bug in tail duplication. I’ve been chasing my tail a bit on this because the code that reports the error is only compiled when exercising the matching code, so I assumed they were causally connected. I just added logic that allows the matching logic to run but never installs the match result, so the code is exercised but doesn’t actually change code generation and it still shows up. Looking at the graph I can see if the isInt guard disappear during tail duplication which causes the problem I’m seeing. https://lafo.ssw.uni-linz.ac.at/jira/browse/GRAAL-791 has a patch that makes it easy to reproduce. Hopefully Lukas can get to the bottom of it.
>
> tom
>
> On May 14, 2014, at 11:01 AM, Tom Rodriguez <tom.rodriguez at oracle.com<mailto:tom.rodriguez at oracle.com>> wrote:
>
> Thanks for the test case. I’ve been narrowing it down in between a few other things but I was seeing an issue with a mirrored unsigned compare. It generates bad code in the compiler itself and then fails much later. It’s possible there are multiple things going on. Anyway, I hope to have it resolved today.
>
> tom
>
> On May 14, 2014, at 10:56 AM, Yudi Zheng <yudi.zheng at usi.ch<mailto:yudi.zheng at usi.ch>> wrote:
>
> In case you might need it, please find attached the simplified test case.
> During the TailDuplicationPhase when compiling TailDuplicatoinTest.bar,
> some ObjectEqualsNode is evaluated to false..
> (com.oracle.graal.nodes.calc.ObjectEqualsNode.java#64)
>
> - Yudi
>
>
> On 13 May 2014, at 00:40, Tom Rodriguez <tom.rodriguez at oracle.com<mailto:tom.rodriguez at oracle.com><mailto:tom.rodriguez at oracle.com>> wrote:
>
> Thanks for the report. I’m looking into it.
>
> tom
>
> On May 12, 2014, at 1:10 PM, Yudi Zheng <yudi.zheng at usi.ch<mailto:yudi.zheng at usi.ch><mailto:yudi.zheng at usi.ch>> wrote:
>
> Hi,
>
> 1. A minor problem is that the default TypeProfileWidth is 2, resulting in lots of megamorphic-inlining with a probability of 60~70% in many cases,
> instead of polymorphic-inlining.
>
> 2. When running with -XX:TypeProfileWidth=8, I observed buggy behavior:
>
> mx vm -XX:+BootstrapGraal -XX:TypeProfileWidth=8 -version
> Bootstrapping Graal.......................com.oracle.graal.compiler.common.GraalInternalError: should not reach here
> at lir instruction: B192 at 1350 LCMP (address: [rax|j + 24], y: long[33286881400|0x7c00d8078]{HotSpotType<Lcom/oracle/graal/nodes/DeoptimizingNode$DeoptBefore;, resolved>}) kind: long
> [B0, B2, B3, B5, B6, B8, B416, B419, B9, B11, B12, B14, B16, B17, B19, B21, B413, B23, B25, B26, B28, B29, B31, B409, B412, B32, B33, B35, B36, B38, B39, B41, B42, B44, B45, B47, B49, B50, B52, B54, B53, B55, B56, B57, B59, B61, B63, B65, B66, B67, B69, B70, B72, B74, B77, B79, B80, B94, B96, B98, B99, B101, B102, B103, B104, B107, B108, B109, B110, B113, B114, B115, B117, B118, B120, B122, B125, B127, B128, B129, B143, B145, B147, B148, B160, B126, B119, B149, B116, B130, B132, B153, B133, B136, B138, B139, B141, B142, B137, B140, B150, B134, B168, B105, B166, B171, B180, B181, B183, B186, B188, B190, B192, B194, B196, B198, B200, B197, B161, B170, B179, B177, B178, B184, B403, B78, B193, B202, B203, B204, B209, B211, B212, B214, B399, B401, B215, B216, B218, B220, B222, B397, B71, B208, B205, B206, B100, B195, B201, B187, B189, B68, B191, B199, B219, B224, B226, B228, B229, B231, B233, B394, B395, B221, B223, B82, B84, B173, B174, B154, B155, B51, B48, B232, B235, B239, B393, B236, B238, B234, B85, B87, B89, B90, B92, B93, B88, B240, B242, B284, B286, B288, B289, B291, B294, B296, B297, B299, B300, B302, B304, B307, B308, B310, B311, B313, B315, B317, B318, B320, B322, B323, B325, B326, B327, B343, B345, B346, B348, B349, B351, B354, B356, B357, B358, B360, B362, B364, B365, B367, B369, B372, B368, B373, B83, B371, B46, B352, B237, B287, B316, B330, B332, B333, B335, B336, B337, B244, B246, B247, B249, B250, B252, B254, B257, B258, B260, B261, B263, B265, B267, B268, B270, B273, B275, B276, B370, B266, B207, B285, B314, B264, B91, B355, B295, B245, B339, B384, B301, B306, B251, B256, B390, B391, B281, B283, B167, B282, B347, B382, B344, B30, B27, B378, B379, B175, B385, B388, B324, B386, B34, B407, B259, B280, B402, B312, B387, B309, B62, B389, B24, B410, B210, B392, B165, B230, B278, B159, B415, B396, B135, B404, B262, B169, B157, B10, B381, B15, B331, B376, B277, B405, B414, B37, B1, B213, B398, B417, B279, B418, B408, B400, B176, B338, B58, B40, B151, B274, B411, B380, B375, B334, B7, B340, B64, B4, B377, B406, B60, B13, B172, B163, B106, B86, B217, B18, B112, B383]
> at com.oracle.graal.compiler.common.GraalInternalError.shouldNotReachHere(GraalInternalError.java:44)
> at com.oracle.graal.lir.amd64.AMD64Compare$CompareMemoryOp.emitMemAccess(AMD64Compare.java:129)
> at com.oracle.graal.lir.amd64.AMD64Move$MemOp.emitCode(AMD64Move.java:129)
> at com.oracle.graal.lir.amd64.AMD64LIRInstruction.emitCode(AMD64LIRInstruction.java:36)
> at com.oracle.graal.lir.asm.CompilationResultBuilder.emitOp(CompilationResultBuilder.java:350)
> at com.oracle.graal.lir.asm.CompilationResultBuilder.emitBlock(CompilationResultBuilder.java:341)
> at com.oracle.graal.lir.asm.CompilationResultBuilder.emit(CompilationResultBuilder.java:323)
> at com.oracle.graal.hotspot.amd64.AMD64HotSpotBackend.emitCodeBody(AMD64HotSpotBackend.java:301)
> at com.oracle.graal.hotspot.amd64.AMD64HotSpotBackend.emitCode(AMD64HotSpotBackend.java:254)
> at com.oracle.graal.compiler.GraalCompiler.emitCode(GraalCompiler.java:288)
> at com.oracle.graal.compiler.GraalCompiler.emitBackEnd(GraalCompiler.java:200)
> at com.oracle.graal.compiler.GraalCompiler.compileGraph(GraalCompiler.java:141)
> at com.oracle.graal.hotspot.CompilationTask.runCompilation(CompilationTask.java:302)
> at com.oracle.graal.hotspot.CompilationTask.run(CompilationTask.java:159)
> at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142)
> at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617)
> at java.lang.Thread.run(Thread.java:745)
> at com.oracle.graal.compiler.CompilerThread.run(CompilerThread.java:48)
>
>
> The potential bug can be tracked in com.oracle.graal.compiler.amd64.AMD64NodeLIRBuilder.emitCompareBranchMemory
> (-G:Dump= -G:MethodFilter=com.oracle.graal.compiler.amd64.AMD64NodeLIRBuilder.emitCompareBranchMemory),
> and can be bypassed either by not inlining com.oracle.graal.compiler.amd64.AMD64NodeLIRBuilder.getMemoryKind,
> or by skipping the TailDuplicationPhase.
>
> I guess that the TailDuplicationPhase dropped some of the IfNode. Could be verified using the following code:
>
> protected ComplexMatchResult emitCompareBranchMemory(IfNode ifNode, CompareNode compare, ValueNode value, Access access) {
> Condition cond = compare.condition();
> Kind kind = getMemoryKind(access);
>
> if (kind != Kind.Long) {
> notInlinedMethodPlaceHolder(kind);
> }
>
> After tail duplication at some merge node, the IfNode<if (kind != Kind.Long)> disappeared..
>
> - Yudi
>
>
>
>
> <TailDuplicationTest.java>
>
>
>
More information about the graal-dev
mailing list