llvm.org GIT mirror llvm / abb2e44
[EarlyCSE] Ensure equal keys have the same hash value Summary: The logic in EarlyCSE that looks through 'not' operations in the predicate recognizes e.g. that `select (not (cmp sgt X, Y)), X, Y` is equivalent to `select (cmp sgt X, Y), Y, X`. Without this change, however, only the latter is recognized as a form of `smin X, Y`, so the two expressions receive different hash codes. This leads to missed optimization opportunities when the quadratic probing for the two hashes doesn't happen to collide, and assertion failures when probing doesn't collide on insertion but does collide on a subsequent table grow operation. This change inverts the order of some of the pattern matching, checking first for the optional `not` and then for the min/max/abs patterns, so that e.g. both expressions above are recognized as a form of `smin X, Y`. It also adds an assertion to isEqual verifying that it implies equal hash codes; this fires when there's a collision during insertion, not just grow, and so will make it easier to notice if these functions fall out of sync again. A new flag --earlycse-debug-hash is added which can be used when changing the hash function; it forces hash collisions so that any pair of values inserted which compare as equal but hash differently will be caught by the isEqual assertion. Reviewers: spatel, nikic Reviewed By: spatel, nikic Subscribers: lebedev.ri, arsenm, craig.topper, efriedma, hiraditya, llvm-commits Tags: #llvm Differential Revision: https://reviews.llvm.org/D62644 git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@363274 91177308-0d34-0410-b5e6-96231b3b80d8 Joseph Tremoulet 3 months ago
4 changed file(s) with 169 addition(s) and 82 deletion(s). Raw diff Collapse all Expand all
605605 return Result;
606606 }
607607
608 /// Determine the pattern that a select with the given compare as its
609 /// predicate and given values as its true/false operands would match.
610 SelectPatternResult matchDecomposedSelectPattern(
611 CmpInst *CmpI, Value *TrueVal, Value *FalseVal, Value *&LHS, Value *&RHS,
612 Instruction::CastOps *CastOp = nullptr, unsigned Depth = 0);
613
608614 /// Return the canonical comparison predicate for the specified
609615 /// minimum/maximum flavor.
610616 CmpInst::Predicate getMinMaxPred(SelectPatternFlavor SPF,
50725072 CmpInst *CmpI = dyn_cast(SI->getCondition());
50735073 if (!CmpI) return {SPF_UNKNOWN, SPNB_NA, false};
50745074
5075 Value *TrueVal = SI->getTrueValue();
5076 Value *FalseVal = SI->getFalseValue();
5077
5078 return llvm::matchDecomposedSelectPattern(CmpI, TrueVal, FalseVal, LHS, RHS,
5079 CastOp, Depth);
5080 }
5081
5082 SelectPatternResult llvm::matchDecomposedSelectPattern(
5083 CmpInst *CmpI, Value *TrueVal, Value *FalseVal, Value *&LHS, Value *&RHS,
5084 Instruction::CastOps *CastOp, unsigned Depth) {
50755085 CmpInst::Predicate Pred = CmpI->getPredicate();
50765086 Value *CmpLHS = CmpI->getOperand(0);
50775087 Value *CmpRHS = CmpI->getOperand(1);
5078 Value *TrueVal = SI->getTrueValue();
5079 Value *FalseVal = SI->getFalseValue();
50805088 FastMathFlags FMF;
50815089 if (isa(CmpI))
50825090 FMF = CmpI->getFastMathFlags();
7979 cl::desc("Enable imprecision in EarlyCSE in pathological cases, in exchange "
8080 "for faster compile. Caps the MemorySSA clobbering calls."));
8181
82 static cl::opt EarlyCSEDebugHash(
83 "earlycse-debug-hash", cl::init(false), cl::Hidden,
84 cl::desc("Perform extra assertion checking to verify that SimpleValue's hash "
85 "function is well-behaved w.r.t. its isEqual predicate"));
86
8287 //===----------------------------------------------------------------------===//
8388 // SimpleValue
8489 //===----------------------------------------------------------------------===//
129134
130135 } // end namespace llvm
131136
132 /// Match a 'select' including an optional 'not' of the condition.
133 static bool matchSelectWithOptionalNotCond(Value *V, Value *&Cond,
134 Value *&T, Value *&F) {
135 if (match(V, m_Select(m_Value(Cond), m_Value(T), m_Value(F)))) {
136 // Look through a 'not' of the condition operand by swapping true/false.
137 Value *CondNot;
138 if (match(Cond, m_Not(m_Value(CondNot)))) {
139 Cond = CondNot;
140 std::swap(T, F);
141 }
142 return true;
143 }
144 return false;
137 /// Match a 'select' including an optional 'not's of the condition.
138 static bool matchSelectWithOptionalNotCond(Value *V, Value *&Cond, Value *&A,
139 Value *&B,
140 SelectPatternFlavor &Flavor) {
141 // Return false if V is not even a select.
142 if (!match(V, m_Select(m_Value(Cond), m_Value(A), m_Value(B))))
143 return false;
144
145 // Look through a 'not' of the condition operand by swapping A/B.
146 Value *CondNot;
147 if (match(Cond, m_Not(m_Value(CondNot)))) {
148 Cond = CondNot;
149 std::swap(A, B);
150 }
151
152 // Set flavor if we find a match, or set it to unknown otherwise; in
153 // either case, return true to indicate that this is a select we can
154 // process.
155 if (auto *CmpI = dyn_cast(Cond))
156 Flavor = matchDecomposedSelectPattern(CmpI, A, B, A, B).Flavor;
157 else
158 Flavor = SPF_UNKNOWN;
159
160 return true;
145161 }
146162
147 unsigned DenseMapInfo::getHashValue(SimpleValue Val) {
163 static unsigned getHashValueImpl(SimpleValue Val) {
148164 Instruction *Inst = Val.Inst;
149165 // Hash in all of the operands as pointers.
150166 if (BinaryOperator *BinOp = dyn_cast(Inst)) {
167183 return hash_combine(Inst->getOpcode(), Pred, LHS, RHS);
168184 }
169185
170 // Hash min/max/abs (cmp + select) to allow for commuted operands.
171 // Min/max may also have non-canonical compare predicate (eg, the compare for
172 // smin may use 'sgt' rather than 'slt'), and non-canonical operands in the
173 // compare.
174 Value *A, *B;
175 SelectPatternFlavor SPF = matchSelectPattern(Inst, A, B).Flavor;
176 // TODO: We should also detect FP min/max.
177 if (SPF == SPF_SMIN || SPF == SPF_SMAX ||
178 SPF == SPF_UMIN || SPF == SPF_UMAX) {
179 if (A > B)
180 std::swap(A, B);
181 return hash_combine(Inst->getOpcode(), SPF, A, B);
182 }
183 if (SPF == SPF_ABS || SPF == SPF_NABS) {
184 // ABS/NABS always puts the input in A and its negation in B.
185 return hash_combine(Inst->getOpcode(), SPF, A, B);
186 }
187
188186 // Hash general selects to allow matching commuted true/false operands.
189 Value *Cond, *TVal, *FVal;
190 if (matchSelectWithOptionalNotCond(Inst, Cond, TVal, FVal)) {
187 SelectPatternFlavor SPF;
188 Value *Cond, *A, *B;
189 if (matchSelectWithOptionalNotCond(Inst, Cond, A, B, SPF)) {
190 // Hash min/max/abs (cmp + select) to allow for commuted operands.
191 // Min/max may also have non-canonical compare predicate (eg, the compare for
192 // smin may use 'sgt' rather than 'slt'), and non-canonical operands in the
193 // compare.
194 // TODO: We should also detect FP min/max.
195 if (SPF == SPF_SMIN || SPF == SPF_SMAX ||
196 SPF == SPF_UMIN || SPF == SPF_UMAX) {
197 if (A > B)
198 std::swap(A, B);
199 return hash_combine(Inst->getOpcode(), SPF, A, B);
200 }
201 if (SPF == SPF_ABS || SPF == SPF_NABS) {
202 // ABS/NABS always puts the input in A and its negation in B.
203 return hash_combine(Inst->getOpcode(), SPF, A, B);
204 }
205
206 // Hash general selects to allow matching commuted true/false operands.
207
191208 // If we do not have a compare as the condition, just hash in the condition.
192209 CmpInst::Predicate Pred;
193210 Value *X, *Y;
194211 if (!match(Cond, m_Cmp(Pred, m_Value(X), m_Value(Y))))
195 return hash_combine(Inst->getOpcode(), Cond, TVal, FVal);
212 return hash_combine(Inst->getOpcode(), Cond, A, B);
196213
197214 // Similar to cmp normalization (above) - canonicalize the predicate value:
198 // select (icmp Pred, X, Y), T, F --> select (icmp InvPred, X, Y), F, T
215 // select (icmp Pred, X, Y), A, B --> select (icmp InvPred, X, Y), B, A
199216 if (CmpInst::getInversePredicate(Pred) < Pred) {
200217 Pred = CmpInst::getInversePredicate(Pred);
201 std::swap(TVal, FVal);
202 }
203 return hash_combine(Inst->getOpcode(), Pred, X, Y, TVal, FVal);
218 std::swap(A, B);
219 }
220 return hash_combine(Inst->getOpcode(), Pred, X, Y, A, B);
204221 }
205222
206223 if (CastInst *CI = dyn_cast(Inst))
226243 hash_combine_range(Inst->value_op_begin(), Inst->value_op_end()));
227244 }
228245
229 bool DenseMapInfo::isEqual(SimpleValue LHS, SimpleValue RHS) {
246 unsigned DenseMapInfo::getHashValue(SimpleValue Val) {
247 #ifndef NDEBUG
248 // If -earlycse-debug-hash was specified, return a constant -- this
249 // will force all hashing to collide, so we'll exhaustively search
250 // the table for a match, and the assertion in isEqual will fire if
251 // there's a bug causing equal keys to hash differently.
252 if (EarlyCSEDebugHash)
253 return 0;
254 #endif
255 return getHashValueImpl(Val);
256 }
257
258 static bool isEqualImpl(SimpleValue LHS, SimpleValue RHS) {
230259 Instruction *LHSI = LHS.Inst, *RHSI = RHS.Inst;
231260
232261 if (LHS.isSentinel() || RHS.isSentinel())
262291
263292 // Min/max/abs can occur with commuted operands, non-canonical predicates,
264293 // and/or non-canonical operands.
265 Value *LHSA, *LHSB;
266 SelectPatternFlavor LSPF = matchSelectPattern(LHSI, LHSA, LHSB).Flavor;
267 // TODO: We should also detect FP min/max.
268 if (LSPF == SPF_SMIN || LSPF == SPF_SMAX ||
269 LSPF == SPF_UMIN || LSPF == SPF_UMAX ||
270 LSPF == SPF_ABS || LSPF == SPF_NABS) {
271 Value *RHSA, *RHSB;
272 SelectPatternFlavor RSPF = matchSelectPattern(RHSI, RHSA, RHSB).Flavor;
294 // Selects can be non-trivially equivalent via inverted conditions and swaps.
295 SelectPatternFlavor LSPF, RSPF;
296 Value *CondL, *CondR, *LHSA, *RHSA, *LHSB, *RHSB;
297 if (matchSelectWithOptionalNotCond(LHSI, CondL, LHSA, LHSB, LSPF) &&
298 matchSelectWithOptionalNotCond(RHSI, CondR, RHSA, RHSB, RSPF)) {
273299 if (LSPF == RSPF) {
274 // Abs results are placed in a defined order by matchSelectPattern.
275 if (LSPF == SPF_ABS || LSPF == SPF_NABS)
300 // TODO: We should also detect FP min/max.
301 if (LSPF == SPF_SMIN || LSPF == SPF_SMAX ||
302 LSPF == SPF_UMIN || LSPF == SPF_UMAX)
303 return ((LHSA == RHSA && LHSB == RHSB) ||
304 (LHSA == RHSB && LHSB == RHSA));
305
306 if (LSPF == SPF_ABS || LSPF == SPF_NABS) {
307 // Abs results are placed in a defined order by matchSelectPattern.
276308 return LHSA == RHSA && LHSB == RHSB;
277 return ((LHSA == RHSA && LHSB == RHSB) ||
278 (LHSA == RHSB && LHSB == RHSA));
279 }
280 }
281
282 // Selects can be non-trivially equivalent via inverted conditions and swaps.
283 Value *CondL, *CondR, *TrueL, *TrueR, *FalseL, *FalseR;
284 if (matchSelectWithOptionalNotCond(LHSI, CondL, TrueL, FalseL) &&
285 matchSelectWithOptionalNotCond(RHSI, CondR, TrueR, FalseR)) {
286 // select Cond, T, F <--> select not(Cond), F, T
287 if (CondL == CondR && TrueL == TrueR && FalseL == FalseR)
288 return true;
309 }
310
311 // select Cond, A, B <--> select not(Cond), B, A
312 if (CondL == CondR && LHSA == RHSA && LHSB == RHSB)
313 return true;
314 }
289315
290316 // If the true/false operands are swapped and the conditions are compares
291317 // with inverted predicates, the selects are equal:
292 // select (icmp Pred, X, Y), T, F <--> select (icmp InvPred, X, Y), F, T
318 // select (icmp Pred, X, Y), A, B <--> select (icmp InvPred, X, Y), B, A
293319 //
294 // This also handles patterns with a double-negation because we looked
295 // through a 'not' in the matching function and swapped T/F:
296 // select (cmp Pred, X, Y), T, F <--> select (not (cmp InvPred, X, Y)), T, F
297 if (TrueL == FalseR && FalseL == TrueR) {
320 // This also handles patterns with a double-negation in the sense of not +
321 // inverse, because we looked through a 'not' in the matching function and
322 // swapped A/B:
323 // select (cmp Pred, X, Y), A, B <--> select (not (cmp InvPred, X, Y)), B, A
324 //
325 // This intentionally does NOT handle patterns with a double-negation in
326 // the sense of not + not, because doing so could result in values
327 // comparing
328 // as equal that hash differently in the min/max/abs cases like:
329 // select (cmp slt, X, Y), X, Y <--> select (not (not (cmp slt, X, Y))), X, Y
330 // ^ hashes as min ^ would not hash as min
331 // In the context of the EarlyCSE pass, however, such cases never reach
332 // this code, as we simplify the double-negation before hashing the second
333 // select (and so still succeed at CSEing them).
334 if (LHSA == RHSB && LHSB == RHSA) {
298335 CmpInst::Predicate PredL, PredR;
299336 Value *X, *Y;
300337 if (match(CondL, m_Cmp(PredL, m_Value(X), m_Value(Y))) &&
305342 }
306343
307344 return false;
345 }
346
347 bool DenseMapInfo::isEqual(SimpleValue LHS, SimpleValue RHS) {
348 // These comparisons are nontrivial, so assert that equality implies
349 // hash equality (DenseMap demands this as an invariant).
350 bool Result = isEqualImpl(LHS, RHS);
351 assert(!Result || (LHS.isSentinel() && LHS.Inst == RHS.Inst) ||
352 getHashValueImpl(LHS) == getHashValueImpl(RHS));
353 return Result;
308354 }
309355
310356 //===----------------------------------------------------------------------===//
0 ; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
1 ; RUN: opt < %s -S -early-cse | FileCheck %s
1 ; RUN: opt < %s -S -early-cse -earlycse-debug-hash | FileCheck %s
22 ; RUN: opt < %s -S -basicaa -early-cse-memssa | FileCheck %s
33
44 define void @test1(float %A, float %B, float* %PA, float* %PB) {
107107 }
108108
109109 ; Min/max can also have an inverted predicate and select operands.
110 ; TODO: Ensure we always recognize this (currently depends on hash collision)
111110
112111 define i1 @smin_inverted(i8 %a, i8 %b) {
113112 ; CHECK-LABEL: @smin_inverted(
114113 ; CHECK-NEXT: [[CMP1:%.*]] = icmp slt i8 [[A:%.*]], [[B:%.*]]
115114 ; CHECK-NEXT: [[CMP2:%.*]] = xor i1 [[CMP1]], true
116115 ; CHECK-NEXT: [[M1:%.*]] = select i1 [[CMP1]], i8 [[A]], i8 [[B]]
117 ; CHECK: ret i1
116 ; CHECK-NEXT: ret i1 true
118117 ;
119118 %cmp1 = icmp slt i8 %a, %b
120119 %cmp2 = xor i1 %cmp1, -1
154153 ret i8 %r
155154 }
156155
157 ; TODO: Ensure we always recognize this (currently depends on hash collision)
158156 define i1 @smax_inverted(i8 %a, i8 %b) {
159157 ; CHECK-LABEL: @smax_inverted(
160158 ; CHECK-NEXT: [[CMP1:%.*]] = icmp sgt i8 [[A:%.*]], [[B:%.*]]
161159 ; CHECK-NEXT: [[CMP2:%.*]] = xor i1 [[CMP1]], true
162160 ; CHECK-NEXT: [[M1:%.*]] = select i1 [[CMP1]], i8 [[A]], i8 [[B]]
163 ; CHECK: ret i1
161 ; CHECK-NEXT: ret i1 true
164162 ;
165163 %cmp1 = icmp sgt i8 %a, %b
166164 %cmp2 = xor i1 %cmp1, -1
202200 ret <2 x i8> %r
203201 }
204202
205 ; TODO: Ensure we always recognize this (currently depends on hash collision)
206203 define i1 @umin_inverted(i8 %a, i8 %b) {
207204 ; CHECK-LABEL: @umin_inverted(
208205 ; CHECK-NEXT: [[CMP1:%.*]] = icmp ult i8 [[A:%.*]], [[B:%.*]]
209206 ; CHECK-NEXT: [[CMP2:%.*]] = xor i1 [[CMP1]], true
210207 ; CHECK-NEXT: [[M1:%.*]] = select i1 [[CMP1]], i8 [[A]], i8 [[B]]
211 ; CHECK: ret i1
208 ; CHECK-NEXT: ret i1 true
212209 ;
213210 %cmp1 = icmp ult i8 %a, %b
214211 %cmp2 = xor i1 %cmp1, -1
249246 ret i8 %r
250247 }
251248
252 ; TODO: Ensure we always recognize this (currently depends on hash collision)
253249 define i1 @umax_inverted(i8 %a, i8 %b) {
254250 ; CHECK-LABEL: @umax_inverted(
255251 ; CHECK-NEXT: [[CMP1:%.*]] = icmp ugt i8 [[A:%.*]], [[B:%.*]]
256252 ; CHECK-NEXT: [[CMP2:%.*]] = xor i1 [[CMP1]], true
257253 ; CHECK-NEXT: [[M1:%.*]] = select i1 [[CMP1]], i8 [[A]], i8 [[B]]
258 ; CHECK: ret i1
254 ; CHECK-NEXT: ret i1 true
259255 ;
260256 %cmp1 = icmp ugt i8 %a, %b
261257 %cmp2 = xor i1 %cmp1, -1
301297 ret i8 %r
302298 }
303299
304 ; TODO: Ensure we always recognize this (currently depends on hash collision)
305300 define i8 @abs_inverted(i8 %a) {
306301 ; CHECK-LABEL: @abs_inverted(
307302 ; CHECK-NEXT: [[NEG:%.*]] = sub i8 0, [[A:%.*]]
308303 ; CHECK-NEXT: [[CMP1:%.*]] = icmp sgt i8 [[A]], 0
309304 ; CHECK-NEXT: [[CMP2:%.*]] = xor i1 [[CMP1]], true
310305 ; CHECK-NEXT: [[M1:%.*]] = select i1 [[CMP1]], i8 [[A]], i8 [[NEG]]
311 ; CHECK: ret i8
306 ; CHECK-NEXT: ret i8 [[M1]]
312307 ;
313308 %neg = sub i8 0, %a
314309 %cmp1 = icmp sgt i8 %a, 0
336331 ret i8 %r
337332 }
338333
339 ; TODO: Ensure we always recognize this (currently depends on hash collision)
340334 define i8 @nabs_inverted(i8 %a) {
341335 ; CHECK-LABEL: @nabs_inverted(
342336 ; CHECK-NEXT: [[NEG:%.*]] = sub i8 0, [[A:%.*]]
343337 ; CHECK-NEXT: [[CMP1:%.*]] = icmp slt i8 [[A]], 0
344338 ; CHECK-NEXT: [[CMP2:%.*]] = xor i1 [[CMP1]], true
345339 ; CHECK-NEXT: [[M1:%.*]] = select i1 [[CMP1]], i8 [[A]], i8 [[NEG]]
346 ; CHECK: ret i8
340 ; CHECK-NEXT: ret i8 0
347341 ;
348342 %neg = sub i8 0, %a
349343 %cmp1 = icmp slt i8 %a, 0
645639 %r = sub i32 %m2, %m1
646640 ret i32 %r
647641 }
642
643
644 ; This test is a reproducer for a bug involving inverted min/max selects
645 ; hashing differently but comparing as equal. It exhibits such a pair of
646 ; values, and we run this test with -earlycse-debug-hash which would catch
647 ; the disagreement and fail if it regressed. This test also includes a
648 ; negation of each negation to check for the same issue one level deeper.
649 define void @not_not_min(i32* %px, i32* %py, i32* %pout) {
650 ; CHECK-LABEL: @not_not_min(
651 ; CHECK-NEXT: [[X:%.*]] = load volatile i32, i32* [[PX:%.*]]
652 ; CHECK-NEXT: [[Y:%.*]] = load volatile i32, i32* [[PY:%.*]]
653 ; CHECK-NEXT: [[CMPA:%.*]] = icmp slt i32 [[X]], [[Y]]
654 ; CHECK-NEXT: [[CMPB:%.*]] = xor i1 [[CMPA]], true
655 ; CHECK-NEXT: [[RA:%.*]] = select i1 [[CMPA]], i32 [[X]], i32 [[Y]]
656 ; CHECK-NEXT: store volatile i32 [[RA]], i32* [[POUT:%.*]]
657 ; CHECK-NEXT: store volatile i32 [[RA]], i32* [[POUT]]
658 ; CHECK-NEXT: store volatile i32 [[RA]], i32* [[POUT]]
659 ; CHECK-NEXT: ret void
660 ;
661 %x = load volatile i32, i32* %px
662 %y = load volatile i32, i32* %py
663 %cmpa = icmp slt i32 %x, %y
664 %cmpb = xor i1 %cmpa, -1
665 %cmpc = xor i1 %cmpb, -1
666 %ra = select i1 %cmpa, i32 %x, i32 %y
667 %rb = select i1 %cmpb, i32 %y, i32 %x
668 %rc = select i1 %cmpc, i32 %x, i32 %y
669 store volatile i32 %ra, i32* %pout
670 store volatile i32 %rb, i32* %pout
671 store volatile i32 %rc, i32* %pout
672
673 ret void
674 }