Skip to content

Commit 3c127e8

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.
1 parent 71bcef0 commit 3c127e8

File tree

2 files changed

+26
-22
lines changed

2 files changed

+26
-22
lines changed

llvm/lib/Transforms/Scalar/ConstraintElimination.cpp

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -366,6 +366,13 @@ struct Decomposition {
366366
append_range(Vars, Other.Vars);
367367
}
368368

369+
void sub(const Decomposition &Other) {
370+
Decomposition Tmp = Other;
371+
Tmp.mul(-1);
372+
add(Tmp.Offset);
373+
append_range(Vars, Tmp.Vars);
374+
}
375+
369376
void mul(int64_t Factor) {
370377
Offset = multiplyWithOverflow(Offset, Factor);
371378
for (auto &Var : Vars)
@@ -569,10 +576,12 @@ static Decomposition decompose(Value *V,
569576
return Result;
570577
}
571578

572-
if (match(V, m_NUWSub(m_Value(Op0), m_ConstantInt(CI))) && canUseSExt(CI))
573-
return {-1 * CI->getSExtValue(), {{1, Op0}}};
574-
if (match(V, m_NUWSub(m_Value(Op0), m_Value(Op1))))
575-
return {0, {{1, Op0}, {-1, Op1}}};
579+
if (match(V, m_NUWSub(m_Value(Op0), m_Value(Op1)))) {
580+
auto ResA = decompose(Op0, Preconditions, IsSigned, DL);
581+
auto ResB = decompose(Op1, Preconditions, IsSigned, DL);
582+
ResA.sub(ResB);
583+
return ResA;
584+
}
576585

577586
return {V, IsKnownNonNegative};
578587
}

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

Lines changed: 13 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -316,12 +316,13 @@ define i1 @sub_nuw_neg_i16(i16 %a) {
316316
; CHECK-LABEL: @sub_nuw_neg_i16(
317317
; CHECK-NEXT: entry:
318318
; CHECK-NEXT: [[NEG2:%.*]] = sub nuw i16 [[A:%.*]], -305
319-
; CHECK-NEXT: br i1 false, label [[EXIT_1:%.*]], label [[EXIT_2:%.*]]
319+
; CHECK-NEXT: [[C_1:%.*]] = icmp ugt i16 0, [[NEG2]]
320+
; CHECK-NEXT: br i1 [[C_1]], label [[EXIT_1:%.*]], label [[EXIT_2:%.*]]
320321
; CHECK: exit.1:
321-
; CHECK-NEXT: ret i1 true
322+
; CHECK-NEXT: [[C_2:%.*]] = icmp ugt i16 [[A]], 0
323+
; CHECK-NEXT: ret i1 [[C_2]]
322324
; CHECK: exit.2:
323-
; CHECK-NEXT: [[C_3:%.*]] = icmp ugt i16 [[A]], 0
324-
; CHECK-NEXT: ret i1 [[C_3]]
325+
; CHECK-NEXT: ret i1 true
325326
;
326327
entry:
327328
%neg2 = sub nuw i16 %a, -305
@@ -380,7 +381,6 @@ entry:
380381
ret i1 %c
381382
}
382383

383-
; FIXME: currently this incorrectly simplifies %c4 to true.
384384
define i1 @pr76713(i16 %i1, i16 %i3) {
385385
; CHECK-LABEL: @pr76713(
386386
; CHECK-NEXT: entry:
@@ -394,7 +394,8 @@ define i1 @pr76713(i16 %i1, i16 %i3) {
394394
; CHECK-NEXT: [[SUB:%.*]] = sub nuw nsw i16 [[I1]], -3
395395
; CHECK-NEXT: [[ARRAYIDX_IDX:%.*]] = mul nuw nsw i16 [[I3]], 4
396396
; CHECK-NEXT: [[I6:%.*]] = add nuw nsw i16 [[ARRAYIDX_IDX]], [[SUB]]
397-
; CHECK-NEXT: ret i1 true
397+
; CHECK-NEXT: [[C4:%.*]] = icmp ult i16 12, [[I6]]
398+
; CHECK-NEXT: ret i1 [[C4]]
398399
; CHECK: else:
399400
; CHECK-NEXT: ret i1 false
400401
;
@@ -417,7 +418,6 @@ else:
417418
ret i1 0
418419
}
419420

420-
; FIXME: Currently gets mis-compiled.
421421
define void @sub_nuw_chained_positive_constants(i16 %a) {
422422
; CHECK-LABEL: @sub_nuw_chained_positive_constants(
423423
; CHECK-NEXT: entry:
@@ -426,16 +426,13 @@ define void @sub_nuw_chained_positive_constants(i16 %a) {
426426
; CHECK-NEXT: [[C_1:%.*]] = icmp ugt i16 [[SUB2]], 90
427427
; CHECK-NEXT: br i1 [[C_1]], label [[EXIT_1:%.*]], label [[EXIT_2:%.*]]
428428
; CHECK: exit.1:
429-
; CHECK-NEXT: [[C_2:%.*]] = icmp ugt i16 [[A]], 120
430-
; CHECK-NEXT: call void @use(i1 [[C_2]])
429+
; CHECK-NEXT: call void @use(i1 true)
431430
; CHECK-NEXT: [[C_3:%.*]] = icmp ugt i16 [[A]], 121
432431
; CHECK-NEXT: call void @use(i1 [[C_3]])
433432
; CHECK-NEXT: ret void
434433
; CHECK: exit.2:
435-
; CHECK-NEXT: [[C_4:%.*]] = icmp ugt i16 [[A]], 120
436-
; CHECK-NEXT: call void @use(i1 [[C_4]])
437-
; CHECK-NEXT: [[C_5:%.*]] = icmp ugt i16 [[A]], 121
438-
; CHECK-NEXT: call void @use(i1 [[C_5]])
434+
; CHECK-NEXT: call void @use(i1 false)
435+
; CHECK-NEXT: call void @use(i1 false)
439436
; CHECK-NEXT: ret void
440437
;
441438
entry:
@@ -459,7 +456,6 @@ exit.2:
459456
ret void
460457
}
461458

462-
; FIXME: Currently gets mis-compiled.
463459
define void @sub_nuw_chained_negative_constants(i8 %a) {
464460
; CHECK-LABEL: @sub_nuw_chained_negative_constants(
465461
; CHECK-NEXT: entry:
@@ -468,14 +464,13 @@ define void @sub_nuw_chained_negative_constants(i8 %a) {
468464
; CHECK-NEXT: [[C_1:%.*]] = icmp ugt i8 [[SUB2]], 20
469465
; CHECK-NEXT: br i1 [[C_1]], label [[EXIT_1:%.*]], label [[EXIT_2:%.*]]
470466
; CHECK: exit.1:
471-
; CHECK-NEXT: [[C_2:%.*]] = icmp ugt i8 [[A]], -96
472-
; CHECK-NEXT: call void @use(i1 [[C_2]])
467+
; CHECK-NEXT: call void @use(i1 true)
473468
; CHECK-NEXT: [[C_3:%.*]] = icmp ugt i8 [[A]], -95
474469
; CHECK-NEXT: call void @use(i1 [[C_3]])
475470
; CHECK-NEXT: ret void
476471
; CHECK: exit.2:
477-
; CHECK-NEXT: call void @use(i1 true)
478-
; CHECK-NEXT: call void @use(i1 true)
472+
; CHECK-NEXT: call void @use(i1 false)
473+
; CHECK-NEXT: call void @use(i1 false)
479474
; CHECK-NEXT: ret void
480475
;
481476
entry:

0 commit comments

Comments
 (0)