Skip to content

Commit 188249f

Browse files
committed
[ConstraintElim] Replace NUWSub decomp with recursive decomp of ops.
The current patterns for NUWSub decompositions do not handle negative constants correctly at the moment (causing llvm#76713). Replace the incorrect pattern by more general code that recursively decomposes the operands and then combines the results. This is already done in most other places that handle operators like add/mul. This means we fall back to the general constant handling code (fixes the mis-compile) while also being able to support reasoning about decomposable expressions in the SUB operands. Fixes llvm#76713. (cherry-picked from 3c127e8)
1 parent 59d8f40 commit 188249f

File tree

2 files changed

+24
-21
lines changed

2 files changed

+24
-21
lines changed

llvm/lib/Transforms/Scalar/ConstraintElimination.cpp

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -358,6 +358,13 @@ struct Decomposition {
358358
append_range(Vars, Other.Vars);
359359
}
360360

361+
void sub(const Decomposition &Other) {
362+
Decomposition Tmp = Other;
363+
Tmp.mul(-1);
364+
add(Tmp.Offset);
365+
append_range(Vars, Tmp.Vars);
366+
}
367+
361368
void mul(int64_t Factor) {
362369
Offset = multiplyWithOverflow(Offset, Factor);
363370
for (auto &Var : Vars)
@@ -542,10 +549,12 @@ static Decomposition decompose(Value *V,
542549
return Result;
543550
}
544551

545-
if (match(V, m_NUWSub(m_Value(Op0), m_ConstantInt(CI))) && canUseSExt(CI))
546-
return {-1 * CI->getSExtValue(), {{1, Op0}}};
547-
if (match(V, m_NUWSub(m_Value(Op0), m_Value(Op1))))
548-
return {0, {{1, Op0}, {-1, Op1}}};
552+
if (match(V, m_NUWSub(m_Value(Op0), m_Value(Op1)))) {
553+
auto ResA = decompose(Op0, Preconditions, IsSigned, DL);
554+
auto ResB = decompose(Op1, Preconditions, IsSigned, DL);
555+
ResA.sub(ResB);
556+
return ResA;
557+
}
549558

550559
return {V, IsKnownNonNegative};
551560
}

llvm/test/Transforms/ConstraintElimination/sub-nuw.ll

Lines changed: 11 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -319,10 +319,10 @@ define i1 @sub_nuw_neg_i16(i16 %a) {
319319
; CHECK-NEXT: [[C_1:%.*]] = icmp ugt i16 0, [[NEG2]]
320320
; CHECK-NEXT: br i1 [[C_1]], label [[EXIT_1:%.*]], label [[EXIT_2:%.*]]
321321
; CHECK: exit.1:
322-
; CHECK-NEXT: ret i1 false
322+
; CHECK-NEXT: [[C_2:%.*]] = icmp ugt i16 [[A]], 0
323+
; CHECK-NEXT: ret i1 [[C_2]]
323324
; CHECK: exit.2:
324-
; CHECK-NEXT: [[C_3:%.*]] = icmp ugt i16 [[A]], 0
325-
; CHECK-NEXT: ret i1 [[C_3]]
325+
; CHECK-NEXT: ret i1 true
326326
;
327327
entry:
328328
%neg2 = sub nuw i16 %a, -305
@@ -381,7 +381,6 @@ entry:
381381
ret i1 %c
382382
}
383383

384-
; FIXME: currently this incorrectly simplifies %c4 to true.
385384
define i1 @pr76713(i16 %i1, i16 %i3) {
386385
; CHECK-LABEL: @pr76713(
387386
; CHECK-NEXT: entry:
@@ -395,7 +394,8 @@ define i1 @pr76713(i16 %i1, i16 %i3) {
395394
; CHECK-NEXT: [[SUB:%.*]] = sub nuw nsw i16 [[I1]], -3
396395
; CHECK-NEXT: [[ARRAYIDX_IDX:%.*]] = mul nuw nsw i16 [[I3]], 4
397396
; CHECK-NEXT: [[I6:%.*]] = add nuw nsw i16 [[ARRAYIDX_IDX]], [[SUB]]
398-
; CHECK-NEXT: ret i1 true
397+
; CHECK-NEXT: [[C4:%.*]] = icmp ult i16 12, [[I6]]
398+
; CHECK-NEXT: ret i1 [[C4]]
399399
; CHECK: else:
400400
; CHECK-NEXT: ret i1 false
401401
;
@@ -418,7 +418,6 @@ else:
418418
ret i1 0
419419
}
420420

421-
; FIXME: Currently gets mis-compiled.
422421
define void @sub_nuw_chained_positive_constants(i16 %a) {
423422
; CHECK-LABEL: @sub_nuw_chained_positive_constants(
424423
; CHECK-NEXT: entry:
@@ -427,16 +426,13 @@ define void @sub_nuw_chained_positive_constants(i16 %a) {
427426
; CHECK-NEXT: [[C_1:%.*]] = icmp ugt i16 [[SUB2]], 90
428427
; CHECK-NEXT: br i1 [[C_1]], label [[EXIT_1:%.*]], label [[EXIT_2:%.*]]
429428
; CHECK: exit.1:
430-
; CHECK-NEXT: [[C_2:%.*]] = icmp ugt i16 [[A]], 120
431-
; CHECK-NEXT: call void @use(i1 [[C_2]])
429+
; CHECK-NEXT: call void @use(i1 true)
432430
; CHECK-NEXT: [[C_3:%.*]] = icmp ugt i16 [[A]], 121
433431
; CHECK-NEXT: call void @use(i1 [[C_3]])
434432
; CHECK-NEXT: ret void
435433
; CHECK: exit.2:
436-
; CHECK-NEXT: [[C_4:%.*]] = icmp ugt i16 [[A]], 120
437-
; CHECK-NEXT: call void @use(i1 [[C_4]])
438-
; CHECK-NEXT: [[C_5:%.*]] = icmp ugt i16 [[A]], 121
439-
; CHECK-NEXT: call void @use(i1 [[C_5]])
434+
; CHECK-NEXT: call void @use(i1 false)
435+
; CHECK-NEXT: call void @use(i1 false)
440436
; CHECK-NEXT: ret void
441437
;
442438
entry:
@@ -460,7 +456,6 @@ exit.2:
460456
ret void
461457
}
462458

463-
; FIXME: Currently gets mis-compiled.
464459
define void @sub_nuw_chained_negative_constants(i8 %a) {
465460
; CHECK-LABEL: @sub_nuw_chained_negative_constants(
466461
; CHECK-NEXT: entry:
@@ -469,14 +464,13 @@ define void @sub_nuw_chained_negative_constants(i8 %a) {
469464
; CHECK-NEXT: [[C_1:%.*]] = icmp ugt i8 [[SUB2]], 20
470465
; CHECK-NEXT: br i1 [[C_1]], label [[EXIT_1:%.*]], label [[EXIT_2:%.*]]
471466
; CHECK: exit.1:
472-
; CHECK-NEXT: [[C_2:%.*]] = icmp ugt i8 [[A]], -96
473-
; CHECK-NEXT: call void @use(i1 [[C_2]])
467+
; CHECK-NEXT: call void @use(i1 true)
474468
; CHECK-NEXT: [[C_3:%.*]] = icmp ugt i8 [[A]], -95
475469
; CHECK-NEXT: call void @use(i1 [[C_3]])
476470
; CHECK-NEXT: ret void
477471
; CHECK: exit.2:
478-
; CHECK-NEXT: call void @use(i1 true)
479-
; CHECK-NEXT: call void @use(i1 true)
472+
; CHECK-NEXT: call void @use(i1 false)
473+
; CHECK-NEXT: call void @use(i1 false)
480474
; CHECK-NEXT: ret void
481475
;
482476
entry:

0 commit comments

Comments
 (0)