Skip to content

Commit

Permalink
[SeparateConstOffsetFromGEP] Fix: b - a matched a - b during reun…
Browse files Browse the repository at this point in the history
…iteExts

During the SeparateConstOffsetFromGEP pass, a - b and b - a will be
considered equivalent in some instances.

An example- the IR contains:

  BB1:
      %add = add %a, 511
      br label %BB2
  BB2:
      %sub2 = sub %b,  %a
      br label %BB3
  BB3:
      %sub1 = sub %add, %b
      %gep = getelementptr float, ptr %p, %sub1

Step 1 in the SeparateConstOffsetFromGEP pass, after split constant index:

  BB1:
      %add = add %a, 511
      br label %BB2
  BB2:
      %sub2 = sub %b,  %a
      br label %BB3
  BB3:
      %sub.t = sub %a, %b
      %gep.base = getelementptr float, ptr %p, %sub.t
      %gep = getelementptr float, ptr %gep.base, 511

Step 2, after reuniteExts:

  BB1:
      br label %BB2
  BB2:
      %sub2 = sub %b,  %a
      br label %BB3
  BB3:
      %gep.base = getelementptr float, ptr %p, %sub2
      %gep = getelementptr float, ptr %gep.base, 511

Obviously, reuniteExts treated a - b and b - a as equivalent.
This patch fixes that.

Reviewed By: nikic, spatel

Differential Revision: https://reviews.llvm.org/D143542
  • Loading branch information
MarsPLR committed Feb 15, 2023
1 parent f93da39 commit 06f0664
Show file tree
Hide file tree
Showing 2 changed files with 13 additions and 11 deletions.
8 changes: 4 additions & 4 deletions llvm/lib/Transforms/Scalar/SeparateConstOffsetFromGEP.cpp
Expand Up @@ -1232,8 +1232,8 @@ bool SeparateConstOffsetFromGEP::reuniteExts(Instruction *I) {
}
} else if (match(I, m_Sub(m_SExt(m_Value(LHS)), m_SExt(m_Value(RHS))))) {
if (LHS->getType() == RHS->getType()) {
const SCEV *Key =
SE->getAddExpr(SE->getUnknown(LHS), SE->getUnknown(RHS));
const SCEV *Key = SE->getAddExpr(
SE->getUnknown(LHS), SE->getNegativeSCEV(SE->getUnknown(RHS)));
if (auto *Dom = findClosestMatchingDominator(Key, I, DominatingSubs)) {
Instruction *NewSExt = new SExtInst(Dom, I->getType(), "", I);
NewSExt->takeName(I);
Expand All @@ -1253,8 +1253,8 @@ bool SeparateConstOffsetFromGEP::reuniteExts(Instruction *I) {
}
} else if (match(I, m_NSWSub(m_Value(LHS), m_Value(RHS)))) {
if (programUndefinedIfPoison(I)) {
const SCEV *Key =
SE->getAddExpr(SE->getUnknown(LHS), SE->getUnknown(RHS));
const SCEV *Key = SE->getAddExpr(
SE->getUnknown(LHS), SE->getNegativeSCEV(SE->getUnknown(RHS)));
DominatingSubs[Key].push_back(I);
}
}
Expand Down
16 changes: 9 additions & 7 deletions llvm/test/Transforms/SeparateConstOffsetFromGEP/split-gep-sub.ll
Expand Up @@ -28,13 +28,15 @@ define void @test_A_sub_B_add_ConstantInt(ptr %p) {
; CHECK-NEXT: [[CMP26:%.*]] = icmp ult i32 [[SUB1]], 512
; CHECK-NEXT: br i1 [[CMP26]], label [[COND_TRUE:%.*]], label [[COND_END]]
; CHECK: cond.true:
; CHECK-NEXT: [[SUB22:%.*]] = sext i32 [[SUB1]] to i64
; CHECK-NEXT: [[TMP1:%.*]] = ptrtoint ptr [[P:%.*]] to i64
; CHECK-NEXT: [[TMP2:%.*]] = shl i64 [[SUB22]], 2
; CHECK-NEXT: [[TMP3:%.*]] = add i64 [[TMP1]], [[TMP2]]
; CHECK-NEXT: [[TMP4:%.*]] = add i64 [[TMP3]], 2044
; CHECK-NEXT: [[TMP5:%.*]] = inttoptr i64 [[TMP4]] to ptr
; CHECK-NEXT: store float 1.000000e+00, ptr [[TMP5]], align 4
; CHECK-NEXT: [[TMP1:%.*]] = sext i32 [[MUL]] to i64
; CHECK-NEXT: [[TMP2:%.*]] = sext i32 [[REM]] to i64
; CHECK-NEXT: [[SUB22:%.*]] = sub i64 [[TMP2]], [[TMP1]]
; CHECK-NEXT: [[TMP3:%.*]] = ptrtoint ptr [[P:%.*]] to i64
; CHECK-NEXT: [[TMP4:%.*]] = shl i64 [[SUB22]], 2
; CHECK-NEXT: [[TMP5:%.*]] = add i64 [[TMP3]], [[TMP4]]
; CHECK-NEXT: [[TMP6:%.*]] = add i64 [[TMP5]], 2044
; CHECK-NEXT: [[TMP7:%.*]] = inttoptr i64 [[TMP6]] to ptr
; CHECK-NEXT: store float 1.000000e+00, ptr [[TMP7]], align 4
; CHECK-NEXT: br label [[COND_END]]
; CHECK: cond.end:
; CHECK-NEXT: [[INC]] = add nuw nsw i32 [[K]], 1
Expand Down

0 comments on commit 06f0664

Please sign in to comment.