[ThinLTO] Fix test with reverse-iteration

[Hahnfeld]

Summary

Make test more flexible with regard to ordering. Additionally, FileCheck
cannot test -SAME patterns after DAG, so duplicate the line manually.

[mehdi_amini]

Is it expected that the test output is sensitive to reverse iteration?
It sounds to me more like an issue with the code than with how the test is expressed.

[evgeny777]

Summary clusters, nodes and edges are retrieved from DenseMap with arbitrary order, which doesn't make result DOT file invalid.
Reverse iteration just exposes test issues.

[evgeny777]

LGTM, but please wait for a while for Teresa to take a look as well

[dexonsmith]

In D58384#1402418, @evgeny777 wrote:

Summary clusters, nodes and edges are retrieved from DenseMap with arbitrary order, which doesn't make result DOT file invalid.
Reverse iteration just exposes test issues.

Does that mean if we change the DenseMap hash, the test output will change? That seems pretty unfortunate. Can you sort the edges/nodes/etc. somehow to avoid this (either before or after dumping)?

[evgeny777]

Can you sort the edges/nodes/etc. somehow to avoid this

It's possible but what for? Just for test case?

[mehdi_amini]

In D58384#1402418, @evgeny777 wrote:

Summary clusters, nodes and edges are retrieved from DenseMap with arbitrary order, which doesn't make result DOT file invalid.
Reverse iteration just exposes test issues.

The dot file is not invalid, but the output is sensitive to "arbitrary order" as you mentioned: this is not desirable. We should strive toward canonicalizing toward an order independent of the container properties.
So instead of iterating a DenseMap and emitting the output, there should be another step that sort the nodes in a deterministic way.

Actually this is already implemented for the bitcode emission I believe, even if the bitcode wouldn't be invalid either. The same logic should also be done for the DOT emission I believe.

[evgeny777]

The same logic should also be done for the DOT emission I believe.

Ok, I'll make a patch

[Hahnfeld]

Closing after D58631 landed which fixes both the bot and passes in my local build. Thanks @evgeny777!

llvm/test/ThinLTO/X86/dot-dumper.ll

 ; RUN: cat %t3.index.dot | FileCheck --check-prefix=STRUCTURE %s
 ; RUN: cat %t3.index.dot | FileCheck --check-prefix=CLUSTER0 --check-prefix=COMBINED0 %s
 ; RUN: cat %t3.index.dot | FileCheck --check-prefix=CLUSTER1 --check-prefix=COMBINED1 %s
 
 ; %t1 index
 ; STRUCTURE1:        digraph Summary {
 ; STRUCTURE1:          subgraph cluster_0
 ; STRUCTURE1:          // Cross-module edges:
-; STRUCTURE1:          0 [label="@0"]; // defined externally
-; STRUCTURE1:          M0_{{[0-9]+}} -> 0 [style=dotted]; // alias
+; STRUCTURE1-DAG:      0 [label="@0"]; // defined externally
+; STRUCTURE1-DAG:      M0_{{[0-9]+}} -> 0 [style=dotted]; // alias
 ; STRUCTURE1-DAG:      [[A:[0-9]+]] [label="A"]; // defined externally
+; STRUCTURE1-DAG:      M0_{{[0-9]+}} -> [[A]] [{{.*}}]; // const-ref
 ; STRUCTURE1-DAG:      [[FOO:[0-9]+]] [label="foo"]; // defined externally
 ; STRUCTURE1-DAG:      M0_{{[0-9]+}} -> [[FOO]] // call
-; STRUCTURE1-DAG:      M0_{{[0-9]+}} -> [[A]] [{{.*}}]; // const-ref
 ; STRUCTURE1-NEXT:   }
 
 ; %t2 index
 ; STRUCTURE2:        digraph Summary {
 ; STRUCTURE2:          subgraph cluster_0
 ; STRUCTURE2:          // Cross-module edges:
 ; STRUCTURE2-NEXT:   }
 

 ; CLUSTER0-DAG:       M[[ID0]]_[[MAIN:[0-9]+]] [{{.*}}main|extern{{.*}}]; // function
 ; CLUSTER0-NEXT:      // Edges:
 ; COMBINED0-NEXT:     M[[ID0]]_[[MAIN_ALIAS]] -> M[[ID0]]_[[MAIN]] [{{.*}}]; // alias
 ; CLUSTER0-NEXT:    }
 
 ; PERMODULE1:       // Module:
 ; COMBINED1:      // Module: {{.*}}2.bc
 ; CLUSTER1-NEXT:    subgraph cluster_[[ID1:[0-1]]] {
-; CLUSTER1-DAG:       M[[ID1]]_[[A:[0-9]+]] [{{.*}}A|extern{{.*}}]; // variable
-; COMBINED1-SAME: , immutable
+; PERMODULE1-DAG:       M[[ID1]]_[[A:[0-9]+]] [{{.*}}A|extern{{.*}}]; // variable
+; COMBINED1-DAG:        M[[ID1]]_[[A:[0-9]+]] [{{.*}}A|extern{{.*}}]; // variable, immutable
 ; CLUSTER1-DAG:       M[[ID1]]_[[FOO:[0-9]+]] [{{.*}}foo|extern{{.*}} ffl: 00001{{.*}}]; // function
-; CLUSTER1-DAG:       M[[ID1]]_[[B:[0-9]+]] [{{.*}}B|extern{{.*}}]; // variable
-; COMBINED1-SAME: , immutable
+; PERMODULE1-DAG:       M[[ID1]]_[[B:[0-9]+]] [{{.*}}B|extern{{.*}}]; // variable
+; COMBINED1-DAG:        M[[ID1]]_[[B:[0-9]+]] [{{.*}}B|extern{{.*}}]; // variable, immutable
 ; CLUSTER1-DAG:       M[[ID1]]_[[BAR:[0-9]+]] [{{.*}}bar|extern{{.*}}]; // function, dead
 ; CLUSTER1-NEXT:      // Edges:
 ; CLUSTER1-DAG:       M[[ID1]]_[[FOO]] -> M[[ID1]]_[[B]] [{{.*}}]; // const-ref
 ; CLUSTER1-DAG:       M[[ID1]]_[[FOO]] -> M[[ID1]]_[[A]] [{{.*}}]; // const-ref
 ; CLUSTER1-DAG:     }
 
 target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
 target triple = "x86_64-unknown-linux-gnu"