llvm.org GIT mirror llvm / b1aec70
Fix non-determinism in Reassociate caused by address coincidences Summary: Between building the pair map and querying it there are a few places that erase and create Values. It's rare but the address of these newly created Values is occasionally the same as a just-erased Value that we already have in the pair map. These coincidences should be accounted for to avoid non-determinism. Thanks to Roman Tereshin for the test case. Reviewers: rtereshin, bogner Reviewed By: rtereshin Subscribers: mgrang, llvm-commits Tags: #llvm Differential Revision: https://reviews.llvm.org/D59401 git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@356803 91177308-0d34-0410-b5e6-96231b3b80d8 Daniel Sanders 5 months ago
3 changed file(s) with 133 addition(s) and 6 deletion(s). Raw diff Collapse all Expand all
8181 static const unsigned GlobalReassociateLimit = 10;
8282 static const unsigned NumBinaryOps =
8383 Instruction::BinaryOpsEnd - Instruction::BinaryOpsBegin;
84 DenseMap, unsigned> PairMap[NumBinaryOps];
84
85 struct PairMapValue {
86 WeakVH Value1;
87 WeakVH Value2;
88 unsigned Score;
89 bool isValid() const { return Value1 && Value2; }
90 };
91 DenseMap, PairMapValue> PairMap[NumBinaryOps];
8592
8693 bool MadeChange;
8794
22162216 if (std::less()(Op1, Op0))
22172217 std::swap(Op0, Op1);
22182218 auto it = PairMap[Idx].find({Op0, Op1});
2219 if (it != PairMap[Idx].end())
2220 Score += it->second;
2219 if (it != PairMap[Idx].end()) {
2220 // Functions like BreakUpSubtract() can erase the Values we're using
2221 // as keys and create new Values after we built the PairMap. There's a
2222 // small chance that the new nodes can have the same address as
2223 // something already in the table. We shouldn't accumulate the stored
2224 // score in that case as it refers to the wrong Value.
2225 if (it->second.isValid())
2226 Score += it->second.Score;
2227 }
22212228
22222229 unsigned MaxRank = std::max(Ops[i].Rank, Ops[j].Rank);
22232230 if (Score > Max || (Score == Max && MaxRank < BestRank)) {
22862293 std::swap(Op0, Op1);
22872294 if (!Visited.insert({Op0, Op1}).second)
22882295 continue;
2289 auto res = PairMap[BinaryIdx].insert({{Op0, Op1}, 1});
2290 if (!res.second)
2291 ++res.first->second;
2296 auto res = PairMap[BinaryIdx].insert({{Op0, Op1}, {Op0, Op1, 1}});
2297 if (!res.second) {
2298 // If either key value has been erased then we've got the same
2299 // address by coincidence. That can't happen here because nothing is
2300 // erasing values but it can happen by the time we're querying the
2301 // map.
2302 assert(res.first->second.isValid() && "WeakVH invalidated");
2303 ++res.first->second.Score;
2304 }
22922305 }
22932306 }
22942307 }
0 ; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
1 ; RUN: opt -run-twice -reassociate %s -S -o - | FileCheck %s
2 ; RUN: opt -run-twice -reassociate %s -S -o - | FileCheck %s
3
4 ; The PairMap[NumBinaryOps] used by the Reassociate pass used to have Value
5 ; *pointers as keys and no handling for values being removed. In some cases (in
6 ; practice very rarely, but in this particular test - well over 50% of the time)
7 ; a newly created Value would happen to get allocated at the same memory
8 ; address, effectively "replacing" the key in the map.
9 ;
10 ; Test that that doesn't happen anymore and the pass is deterministic.
11 ;
12 ; The failure rate of this test (at least, on my 8 core iMac), when ran in the
13 ; context of other unit tests executed | specifically, I was trying
14 ;
15 ; ./bin/llvm-lit -v ../test/Transforms/Reassociate
16 ;
17 ; is as follows:
18 ;
19 ; # of RUN lines repeated | just -run-twice | -run-twice and CHECK lines
20 ; ------------------------+-----------------+---------------------------
21 ; 1 | 30% |
22 ; 2 | 55% | 95%
23 ; 3 | 55% |
24 ;
25 ; hence the specific shape of this test. The IR itself comes from a real-world
26 ; code, successfully bugpointed.
27
28 define float @test(float %arg) {
29 ; CHECK-LABEL: @test(
30 ; CHECK-NEXT: entry:
31 ; CHECK-NEXT: [[TMP:%.*]] = fmul fast float [[ARG:%.*]], 0x3FE99999A0000000
32 ; CHECK-NEXT: [[TMP110:%.*]] = fsub fast float 1.000000e+00, [[TMP]]
33 ; CHECK-NEXT: [[TMP2:%.*]] = fmul fast float [[ARG]], 0x3FE99999A0000000
34 ; CHECK-NEXT: [[TMP311:%.*]] = fsub fast float 1.000000e+00, [[TMP2]]
35 ; CHECK-NEXT: [[REASS_MUL160:%.*]] = fmul fast float [[TMP110]], [[ARG]]
36 ; CHECK-NEXT: [[TMP4:%.*]] = fmul fast float [[REASS_MUL160]], [[TMP311]]
37 ; CHECK-NEXT: [[TMP5:%.*]] = fadd fast float [[TMP4]], [[ARG]]
38 ; CHECK-NEXT: [[TMP6:%.*]] = fmul fast float [[TMP5]], [[ARG]]
39 ; CHECK-NEXT: [[TMP7:%.*]] = fadd fast float [[TMP6]], [[ARG]]
40 ; CHECK-NEXT: [[TMP8:%.*]] = fmul fast float [[TMP7]], [[ARG]]
41 ; CHECK-NEXT: [[TMP9:%.*]] = fadd fast float [[TMP8]], [[ARG]]
42 ; CHECK-NEXT: [[TMP10:%.*]] = fmul fast float [[TMP9]], [[ARG]]
43 ; CHECK-NEXT: [[TMP11:%.*]] = fadd fast float [[TMP10]], [[ARG]]
44 ; CHECK-NEXT: [[TMP12:%.*]] = fmul fast float [[TMP11]], [[ARG]]
45 ; CHECK-NEXT: [[TMP13:%.*]] = fadd fast float [[TMP12]], [[ARG]]
46 ; CHECK-NEXT: [[TMP14:%.*]] = fmul fast float [[TMP13]], [[ARG]]
47 ; CHECK-NEXT: [[TMP15:%.*]] = fadd fast float [[TMP14]], [[ARG]]
48 ; CHECK-NEXT: [[TMP16:%.*]] = fmul fast float [[TMP15]], [[ARG]]
49 ; CHECK-NEXT: [[TMP17:%.*]] = fadd fast float [[TMP16]], [[ARG]]
50 ; CHECK-NEXT: [[TMP18:%.*]] = fmul fast float [[TMP17]], [[ARG]]
51 ; CHECK-NEXT: [[TMP19:%.*]] = fadd fast float [[TMP18]], [[ARG]]
52 ; CHECK-NEXT: [[TMP20:%.*]] = fmul fast float [[TMP19]], [[ARG]]
53 ; CHECK-NEXT: [[TMP21:%.*]] = fadd fast float [[TMP20]], [[ARG]]
54 ; CHECK-NEXT: [[TMP22:%.*]] = fmul fast float [[TMP21]], [[ARG]]
55 ; CHECK-NEXT: [[TMP23:%.*]] = fadd fast float [[TMP22]], [[ARG]]
56 ; CHECK-NEXT: [[REASS_MUL166:%.*]] = fmul fast float [[ARG]], [[ARG]]
57 ; CHECK-NEXT: [[TMP24:%.*]] = fmul fast float [[REASS_MUL166]], [[TMP23]]
58 ; CHECK-NEXT: [[TMP25:%.*]] = fadd fast float [[TMP24]], [[ARG]]
59 ; CHECK-NEXT: [[TMP26:%.*]] = fmul fast float [[TMP25]], [[ARG]]
60 ; CHECK-NEXT: [[TMP27:%.*]] = fadd fast float [[TMP26]], [[ARG]]
61 ; CHECK-NEXT: [[TMP29:%.*]] = fmul fast float [[ARG]], [[ARG]]
62 ; CHECK-NEXT: [[TMP31:%.*]] = fmul fast float [[TMP29]], 0x3FEA2E8B80000000
63 ; CHECK-NEXT: [[TMP33:%.*]] = fmul fast float [[TMP31]], [[TMP27]]
64 ; CHECK-NEXT: [[TMP34:%.*]] = fadd fast float [[TMP33]], [[ARG]]
65 ; CHECK-NEXT: ret float [[TMP34]]
66 ;
67 entry:
68 %tmp = fmul fast float %arg, 0xBFE99999A0000000
69 %tmp1 = fadd fast float %tmp, 1.000000e+00
70 %tmp2 = fmul fast float %arg, 0xBFE99999A0000000
71 %tmp3 = fadd fast float %tmp2, 1.000000e+00
72 %reass.mul156 = fmul fast float %arg, %tmp1
73 %reass.mul160 = fmul fast float %arg, %tmp1
74 %tmp4 = fmul fast float %reass.mul160, %tmp3
75 %tmp5 = fadd fast float %arg, %tmp4
76 %tmp6 = fmul fast float %tmp5, %arg
77 %tmp7 = fadd fast float %tmp6, %arg
78 %tmp8 = fmul fast float %tmp7, %arg
79 %tmp9 = fadd fast float %arg, %tmp8
80 %tmp10 = fmul fast float %tmp9, %arg
81 %tmp11 = fadd fast float %tmp10, %arg
82 %tmp12 = fmul fast float %tmp11, %arg
83 %tmp13 = fadd fast float %tmp12, %arg
84 %tmp14 = fmul fast float %tmp13, %arg
85 %tmp15 = fadd fast float %arg, %tmp14
86 %tmp16 = fmul fast float %tmp15, %arg
87 %tmp17 = fadd fast float %tmp16, %arg
88 %tmp18 = fmul fast float %tmp17, %arg
89 %tmp19 = fadd fast float %tmp18, %arg
90 %tmp20 = fmul fast float %tmp19, %arg
91 %tmp21 = fadd fast float %tmp20, %arg
92 %tmp22 = fmul fast float %tmp21, %arg
93 %tmp23 = fadd fast float %tmp22, %arg
94 %reass.mul166 = fmul fast float %arg, %tmp23
95 %tmp24 = fmul fast float %reass.mul166, %arg
96 %tmp25 = fadd fast float %arg, %tmp24
97 %tmp26 = fmul fast float %arg, %tmp25
98 %tmp27 = fadd fast float %tmp26, %arg
99 %tmp28 = fmul fast float %arg, %tmp27
100 %tmp29 = fmul fast float %tmp28, %arg
101 %tmp31 = fmul fast float %tmp29, 0x3FED1745C0000000
102 %tmp33 = fmul fast float %tmp31, 0x3FECCCCCC0000000
103 %tmp34 = fadd fast float %arg, %tmp33
104 ret float %tmp34
105 }
106