Skip to content
This repository was archived by the owner on Feb 5, 2019. It is now read-only.

Commit 492b2bc

Browse files
zmodemzmodem
committed
Merging r309945, r310779 and r310842:
------------------------------------------------------------------------ r309945 | spatel | 2017-08-03 08:07:37 -0700 (Thu, 03 Aug 2017) | 2 lines [BDCE] add tests to show invalid/incomplete transforms ------------------------------------------------------------------------ ------------------------------------------------------------------------ r310779 | spatel | 2017-08-12 09:41:08 -0700 (Sat, 12 Aug 2017) | 13 lines [BDCE] clear poison generators after turning a value into zero (PR33695, PR34037) nsw, nuw, and exact carry implicit assumptions about their operands, so we need to clear those after trivializing a value. We decided there was no danger for llvm.assume or metadata, so there's just a comment about that. This fixes miscompiles as shown in: https://bugs.llvm.org/show_bug.cgi?id=33695 https://bugs.llvm.org/show_bug.cgi?id=34037 Differential Revision: https://reviews.llvm.org/D36592 ------------------------------------------------------------------------ ------------------------------------------------------------------------ r310842 | spatel | 2017-08-14 08:13:46 -0700 (Mon, 14 Aug 2017) | 16 lines [BDCE] reduce scope of an assert (PR34179) The assert was added with r310779 and is usually correct, but as the test shows, not always. The 'volatile' on the load is needed to expose the faulty path because without it, DemandedBits would return that the load is just dead rather than not demanded, and so we wouldn't hit the bogus assert. Also, since the lambda is just a single-line now, get rid of it and inline the DB.isAllOnesValue() calls. This should fix (prevent execution of a faulty assert): https://bugs.llvm.org/show_bug.cgi?id=34179 ------------------------------------------------------------------------ git-svn-id: https://llvm.org/svn/llvm-project/llvm/branches/release_50@310898 91177308-0d34-0410-b5e6-96231b3b80d8
1 parent 4e549e7 commit 492b2bc

File tree

2 files changed

+144
-0
lines changed

2 files changed

+144
-0
lines changed

‎lib/Transforms/Scalar/BDCE.cpp

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
//===----------------------------------------------------------------------===//
1616

1717
#include "llvm/Transforms/Scalar/BDCE.h"
18+
#include "llvm/ADT/SmallPtrSet.h"
1819
#include "llvm/ADT/SmallVector.h"
1920
#include "llvm/ADT/Statistic.h"
2021
#include "llvm/Analysis/DemandedBits.h"
@@ -35,6 +36,46 @@ using namespace llvm;
3536
STATISTIC(NumRemoved, "Number of instructions removed (unused)");
3637
STATISTIC(NumSimplified, "Number of instructions trivialized (dead bits)");
3738

39+
/// If an instruction is trivialized (dead), then the chain of users of that
40+
/// instruction may need to be cleared of assumptions that can no longer be
41+
/// guaranteed correct.
42+
static void clearAssumptionsOfUsers(Instruction *I, DemandedBits &DB) {
43+
assert(I->getType()->isIntegerTy() && "Trivializing a non-integer value?");
44+
45+
// Initialize the worklist with eligible direct users.
46+
SmallVector<Instruction *, 16> WorkList;
47+
for (User *JU : I->users()) {
48+
// If all bits of a user are demanded, then we know that nothing below that
49+
// in the def-use chain needs to be changed.
50+
auto *J = dyn_cast<Instruction>(JU);
51+
if (J && !DB.getDemandedBits(J).isAllOnesValue())
52+
WorkList.push_back(J);
53+
}
54+
55+
// DFS through subsequent users while tracking visits to avoid cycles.
56+
SmallPtrSet<Instruction *, 16> Visited;
57+
while (!WorkList.empty()) {
58+
Instruction *J = WorkList.pop_back_val();
59+
60+
// NSW, NUW, and exact are based on operands that might have changed.
61+
J->dropPoisonGeneratingFlags();
62+
63+
// We do not have to worry about llvm.assume or range metadata:
64+
// 1. llvm.assume demands its operand, so trivializing can't change it.
65+
// 2. range metadata only applies to memory accesses which demand all bits.
66+
67+
Visited.insert(J);
68+
69+
for (User *KU : J->users()) {
70+
// If all bits of a user are demanded, then we know that nothing below
71+
// that in the def-use chain needs to be changed.
72+
auto *K = dyn_cast<Instruction>(KU);
73+
if (K && !Visited.count(K) && !DB.getDemandedBits(K).isAllOnesValue())
74+
WorkList.push_back(K);
75+
}
76+
}
77+
}
78+
3879
static bool bitTrackingDCE(Function &F, DemandedBits &DB) {
3980
SmallVector<Instruction*, 128> Worklist;
4081
bool Changed = false;
@@ -51,6 +92,9 @@ static bool bitTrackingDCE(Function &F, DemandedBits &DB) {
5192
// replacing all uses with something else. Then, if they don't need to
5293
// remain live (because they have side effects, etc.) we can remove them.
5394
DEBUG(dbgs() << "BDCE: Trivializing: " << I << " (all bits dead)\n");
95+
96+
clearAssumptionsOfUsers(&I, DB);
97+
5498
// FIXME: In theory we could substitute undef here instead of zero.
5599
// This should be reconsidered once we settle on the semantics of
56100
// undef, poison, etc.

‎test/Transforms/BDCE/invalidate-assumptions.ll

Lines changed: 100 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,100 @@
1+
; RUN: opt -bdce %s -S | FileCheck %s
2+
3+
; The 'nuw' on the subtract allows us to deduce that %setbit is not demanded.
4+
; But if we change that value to '0', then the 'nuw' is no longer valid. If we don't
5+
; remove the 'nuw', another pass (-instcombine) may make a transform based on an
6+
; that incorrect assumption and we can miscompile:
7+
; https://bugs.llvm.org/show_bug.cgi?id=33695
8+
9+
define i1 @PR33695(i1 %b, i8 %x) {
10+
; CHECK-LABEL: @PR33695(
11+
; CHECK-NEXT: [[SETBIT:%.*]] = or i8 %x, 64
12+
; CHECK-NEXT: [[LITTLE_NUMBER:%.*]] = zext i1 %b to i8
13+
; CHECK-NEXT: [[BIG_NUMBER:%.*]] = shl i8 0, 1
14+
; CHECK-NEXT: [[SUB:%.*]] = sub i8 [[BIG_NUMBER]], [[LITTLE_NUMBER]]
15+
; CHECK-NEXT: [[TRUNC:%.*]] = trunc i8 [[SUB]] to i1
16+
; CHECK-NEXT: ret i1 [[TRUNC]]
17+
;
18+
%setbit = or i8 %x, 64
19+
%little_number = zext i1 %b to i8
20+
%big_number = shl i8 %setbit, 1
21+
%sub = sub nuw i8 %big_number, %little_number
22+
%trunc = trunc i8 %sub to i1
23+
ret i1 %trunc
24+
}
25+
26+
; Similar to above, but now with more no-wrap.
27+
; https://bugs.llvm.org/show_bug.cgi?id=34037
28+
29+
define i64 @PR34037(i64 %m, i32 %r, i64 %j, i1 %b, i32 %k, i64 %p) {
30+
; CHECK-LABEL: @PR34037(
31+
; CHECK-NEXT: [[CONV:%.*]] = zext i32 %r to i64
32+
; CHECK-NEXT: [[AND:%.*]] = and i64 %m, 0
33+
; CHECK-NEXT: [[NEG:%.*]] = xor i64 0, 34359738367
34+
; CHECK-NEXT: [[OR:%.*]] = or i64 %j, 0
35+
; CHECK-NEXT: [[SHL:%.*]] = shl i64 0, 29
36+
; CHECK-NEXT: [[CONV1:%.*]] = select i1 %b, i64 7, i64 0
37+
; CHECK-NEXT: [[SUB:%.*]] = sub i64 [[SHL]], [[CONV1]]
38+
; CHECK-NEXT: [[CONV2:%.*]] = zext i32 %k to i64
39+
; CHECK-NEXT: [[MUL:%.*]] = mul i64 [[SUB]], [[CONV2]]
40+
; CHECK-NEXT: [[CONV4:%.*]] = and i64 %p, 65535
41+
; CHECK-NEXT: [[AND5:%.*]] = and i64 [[MUL]], [[CONV4]]
42+
; CHECK-NEXT: ret i64 [[AND5]]
43+
;
44+
%conv = zext i32 %r to i64
45+
%and = and i64 %m, %conv
46+
%neg = xor i64 %and, 34359738367
47+
%or = or i64 %j, %neg
48+
%shl = shl i64 %or, 29
49+
%conv1 = select i1 %b, i64 7, i64 0
50+
%sub = sub nuw nsw i64 %shl, %conv1
51+
%conv2 = zext i32 %k to i64
52+
%mul = mul nsw i64 %sub, %conv2
53+
%conv4 = and i64 %p, 65535
54+
%and5 = and i64 %mul, %conv4
55+
ret i64 %and5
56+
}
57+
58+
; This is a manufactured example based on the 1st test to prove that the
59+
; assumption-killing algorithm stops at the call. Ie, it does not remove
60+
; nsw/nuw from the 'add' because a call demands all bits of its argument.
61+
62+
declare i1 @foo(i1)
63+
64+
define i1 @poison_on_call_user_is_ok(i1 %b, i8 %x) {
65+
; CHECK-LABEL: @poison_on_call_user_is_ok(
66+
; CHECK-NEXT: [[SETBIT:%.*]] = or i8 %x, 64
67+
; CHECK-NEXT: [[LITTLE_NUMBER:%.*]] = zext i1 %b to i8
68+
; CHECK-NEXT: [[BIG_NUMBER:%.*]] = shl i8 0, 1
69+
; CHECK-NEXT: [[SUB:%.*]] = sub i8 [[BIG_NUMBER]], [[LITTLE_NUMBER]]
70+
; CHECK-NEXT: [[TRUNC:%.*]] = trunc i8 [[SUB]] to i1
71+
; CHECK-NEXT: [[CALL_RESULT:%.*]] = call i1 @foo(i1 [[TRUNC]])
72+
; CHECK-NEXT: [[ADD:%.*]] = add nuw nsw i1 [[CALL_RESULT]], true
73+
; CHECK-NEXT: [[MUL:%.*]] = mul i1 [[TRUNC]], [[ADD]]
74+
; CHECK-NEXT: ret i1 [[MUL]]
75+
;
76+
%setbit = or i8 %x, 64
77+
%little_number = zext i1 %b to i8
78+
%big_number = shl i8 %setbit, 1
79+
%sub = sub nuw i8 %big_number, %little_number
80+
%trunc = trunc i8 %sub to i1
81+
%call_result = call i1 @foo(i1 %trunc)
82+
%add = add nsw nuw i1 %call_result, 1
83+
%mul = mul i1 %trunc, %add
84+
ret i1 %mul
85+
}
86+
87+
88+
; We were asserting that all users of a trivialized integer-type instruction were
89+
; also integer-typed, but that's too strong. The alloca has a pointer-type result.
90+
91+
define void @PR34179(i32* %a) {
92+
; CHECK-LABEL: @PR34179(
93+
; CHECK-NEXT: [[T0:%.*]] = load volatile i32, i32* %a
94+
; CHECK-NEXT: ret void
95+
;
96+
%t0 = load volatile i32, i32* %a
97+
%vla = alloca i32, i32 %t0
98+
ret void
99+
}
100+

0 commit comments

Comments
 (0)