[ValueTracking] emit a remark when we detect a conflicting assumption (PR31809)

This is a follow-up to D29395 where we try to be good citizens and let the user know that
we've probably gone off the rails.

This should allow us to resolve:
https://llvm.org/bugs/show_bug.cgi?id=31809

Differential Revision: https://reviews.llvm.org/D29404

llvm-svn: 294208
This commit is contained in:
Sanjay Patel 2017-02-06 18:26:06 +00:00
parent 193f7a4566
commit 00cf3d4d68
6 changed files with 73 additions and 25 deletions

View File

@ -42,6 +42,7 @@ namespace llvm {
class Instruction;
class DataLayout;
class FastMathFlags;
class OptimizationRemarkEmitter;
class TargetLibraryInfo;
class Type;
class Value;
@ -296,7 +297,8 @@ namespace llvm {
Value *SimplifyInstruction(Instruction *I, const DataLayout &DL,
const TargetLibraryInfo *TLI = nullptr,
const DominatorTree *DT = nullptr,
AssumptionCache *AC = nullptr);
AssumptionCache *AC = nullptr,
OptimizationRemarkEmitter *ORE = nullptr);
/// Replace all uses of 'I' with 'SimpleV' and simplify the uses recursively.
///

View File

@ -31,6 +31,7 @@ template <typename T> class ArrayRef;
class Instruction;
class Loop;
class LoopInfo;
class OptimizationRemarkEmitter;
class MDNode;
class StringRef;
class TargetLibraryInfo;
@ -52,7 +53,8 @@ template <typename T> class ArrayRef;
const DataLayout &DL, unsigned Depth = 0,
AssumptionCache *AC = nullptr,
const Instruction *CxtI = nullptr,
const DominatorTree *DT = nullptr);
const DominatorTree *DT = nullptr,
OptimizationRemarkEmitter *ORE = nullptr);
/// Compute known bits from the range metadata.
/// \p KnownZero the set of bits that are known to be zero
/// \p KnownOne the set of bits that are known to be one

View File

@ -24,6 +24,7 @@
#include "llvm/Analysis/CaptureTracking.h"
#include "llvm/Analysis/ConstantFolding.h"
#include "llvm/Analysis/MemoryBuiltins.h"
#include "llvm/Analysis/OptimizationDiagnosticInfo.h"
#include "llvm/Analysis/ValueTracking.h"
#include "llvm/Analysis/VectorUtils.h"
#include "llvm/IR/ConstantRange.h"
@ -4452,7 +4453,8 @@ Value *llvm::SimplifyCall(Value *V, ArrayRef<Value *> Args,
/// If not, this returns null.
Value *llvm::SimplifyInstruction(Instruction *I, const DataLayout &DL,
const TargetLibraryInfo *TLI,
const DominatorTree *DT, AssumptionCache *AC) {
const DominatorTree *DT, AssumptionCache *AC,
OptimizationRemarkEmitter *ORE) {
Value *Result;
switch (I->getOpcode()) {
@ -4601,7 +4603,7 @@ Value *llvm::SimplifyInstruction(Instruction *I, const DataLayout &DL,
unsigned BitWidth = I->getType()->getScalarSizeInBits();
APInt KnownZero(BitWidth, 0);
APInt KnownOne(BitWidth, 0);
computeKnownBits(I, KnownZero, KnownOne, DL, /*Depth*/0, AC, I, DT);
computeKnownBits(I, KnownZero, KnownOne, DL, /*Depth*/0, AC, I, DT, ORE);
if ((KnownZero | KnownOne).isAllOnesValue())
Result = ConstantInt::get(I->getType(), KnownOne);
}

View File

@ -20,6 +20,7 @@
#include "llvm/Analysis/MemoryBuiltins.h"
#include "llvm/Analysis/Loads.h"
#include "llvm/Analysis/LoopInfo.h"
#include "llvm/Analysis/OptimizationDiagnosticInfo.h"
#include "llvm/Analysis/VectorUtils.h"
#include "llvm/IR/CallSite.h"
#include "llvm/IR/ConstantRange.h"
@ -76,6 +77,9 @@ struct Query {
AssumptionCache *AC;
const Instruction *CxtI;
const DominatorTree *DT;
// Unlike the other analyses, this may be a nullptr because not all clients
// provide it currently.
OptimizationRemarkEmitter *ORE;
/// Set of assumptions that should be excluded from further queries.
/// This is because of the potential for mutual recursion to cause
@ -90,11 +94,12 @@ struct Query {
unsigned NumExcluded;
Query(const DataLayout &DL, AssumptionCache *AC, const Instruction *CxtI,
const DominatorTree *DT)
: DL(DL), AC(AC), CxtI(CxtI), DT(DT), NumExcluded(0) {}
const DominatorTree *DT, OptimizationRemarkEmitter *ORE = nullptr)
: DL(DL), AC(AC), CxtI(CxtI), DT(DT), ORE(ORE), NumExcluded(0) {}
Query(const Query &Q, const Value *NewExcl)
: DL(Q.DL), AC(Q.AC), CxtI(Q.CxtI), DT(Q.DT), NumExcluded(Q.NumExcluded) {
: DL(Q.DL), AC(Q.AC), CxtI(Q.CxtI), DT(Q.DT), ORE(Q.ORE),
NumExcluded(Q.NumExcluded) {
Excluded = Q.Excluded;
Excluded[NumExcluded++] = NewExcl;
assert(NumExcluded <= Excluded.size());
@ -131,9 +136,10 @@ static void computeKnownBits(const Value *V, APInt &KnownZero, APInt &KnownOne,
void llvm::computeKnownBits(const Value *V, APInt &KnownZero, APInt &KnownOne,
const DataLayout &DL, unsigned Depth,
AssumptionCache *AC, const Instruction *CxtI,
const DominatorTree *DT) {
const DominatorTree *DT,
OptimizationRemarkEmitter *ORE) {
::computeKnownBits(V, KnownZero, KnownOne, Depth,
Query(DL, AC, safeCxtI(V, CxtI), DT));
Query(DL, AC, safeCxtI(V, CxtI), DT, ORE));
}
bool llvm::haveNoCommonBitsSet(const Value *LHS, const Value *RHS,
@ -790,16 +796,21 @@ static void computeKnownBitsFromAssume(const Value *V, APInt &KnownZero,
}
// If assumptions conflict with each other or previous known bits, then we
// have a logical fallacy. This should only happen when a program has
// undefined behavior. We can't assert/crash, so clear out the known bits and
// hope for the best.
// FIXME: Publish a warning/remark that we have encountered UB or the compiler
// is broken.
// have a logical fallacy. It's possible that the assumption is not reachable,
// so this isn't a real bug. On the other hand, the program may have undefined
// behavior, or we might have a bug in the compiler. We can't assert/crash, so
// clear out the known bits, try to warn the user, and hope for the best.
if ((KnownZero & KnownOne) != 0) {
KnownZero.clearAllBits();
KnownOne.clearAllBits();
if (Q.ORE) {
auto *CxtI = const_cast<Instruction *>(Q.CxtI);
OptimizationRemarkAnalysis ORA("value-tracking", "BadAssumption", CxtI);
Q.ORE->emit(ORA << "Detected conflicting code assumptions. Program may "
"have undefined behavior, or compiler may have "
"internal error.");
}
}
}

View File

@ -20,6 +20,7 @@
#include "llvm/ADT/Statistic.h"
#include "llvm/Analysis/AssumptionCache.h"
#include "llvm/Analysis/InstructionSimplify.h"
#include "llvm/Analysis/OptimizationDiagnosticInfo.h"
#include "llvm/Analysis/TargetLibraryInfo.h"
#include "llvm/IR/DataLayout.h"
#include "llvm/IR/Dominators.h"
@ -35,7 +36,8 @@ using namespace llvm;
STATISTIC(NumSimplified, "Number of redundant instructions removed");
static bool runImpl(Function &F, const DominatorTree *DT,
const TargetLibraryInfo *TLI, AssumptionCache *AC) {
const TargetLibraryInfo *TLI, AssumptionCache *AC,
OptimizationRemarkEmitter *ORE) {
const DataLayout &DL = F.getParent()->getDataLayout();
SmallPtrSet<const Instruction *, 8> S1, S2, *ToSimplify = &S1, *Next = &S2;
bool Changed = false;
@ -54,7 +56,7 @@ static bool runImpl(Function &F, const DominatorTree *DT,
// Don't waste time simplifying unused instructions.
if (!I->use_empty()) {
if (Value *V = SimplifyInstruction(I, DL, TLI, DT, AC)) {
if (Value *V = SimplifyInstruction(I, DL, TLI, DT, AC, ORE)) {
// Mark all uses for resimplification next time round the loop.
for (User *U : I->users())
Next->insert(cast<Instruction>(U));
@ -95,6 +97,7 @@ namespace {
AU.addRequired<DominatorTreeWrapperPass>();
AU.addRequired<AssumptionCacheTracker>();
AU.addRequired<TargetLibraryInfoWrapperPass>();
AU.addRequired<OptimizationRemarkEmitterWrapperPass>();
}
/// runOnFunction - Remove instructions that simplify.
@ -108,7 +111,10 @@ namespace {
&getAnalysis<TargetLibraryInfoWrapperPass>().getTLI();
AssumptionCache *AC =
&getAnalysis<AssumptionCacheTracker>().getAssumptionCache(F);
return runImpl(F, DT, TLI, AC);
OptimizationRemarkEmitter *ORE =
&getAnalysis<OptimizationRemarkEmitterWrapperPass>().getORE();
return runImpl(F, DT, TLI, AC, ORE);
}
};
}
@ -119,6 +125,7 @@ INITIALIZE_PASS_BEGIN(InstSimplifier, "instsimplify",
INITIALIZE_PASS_DEPENDENCY(AssumptionCacheTracker)
INITIALIZE_PASS_DEPENDENCY(DominatorTreeWrapperPass)
INITIALIZE_PASS_DEPENDENCY(TargetLibraryInfoWrapperPass)
INITIALIZE_PASS_DEPENDENCY(OptimizationRemarkEmitterWrapperPass)
INITIALIZE_PASS_END(InstSimplifier, "instsimplify",
"Remove redundant instructions", false, false)
char &llvm::InstructionSimplifierID = InstSimplifier::ID;
@ -133,7 +140,8 @@ PreservedAnalyses InstSimplifierPass::run(Function &F,
auto &DT = AM.getResult<DominatorTreeAnalysis>(F);
auto &TLI = AM.getResult<TargetLibraryAnalysis>(F);
auto &AC = AM.getResult<AssumptionAnalysis>(F);
bool Changed = runImpl(F, &DT, &TLI, &AC);
auto &ORE = AM.getResult<OptimizationRemarkEmitterAnalysis>(F);
bool Changed = runImpl(F, &DT, &TLI, &AC, &ORE);
if (!Changed)
return PreservedAnalyses::all();

View File

@ -1,5 +1,10 @@
; NOTE: Assertions have been autogenerated by update_test_checks.py
; RUN: opt -instsimplify -S < %s | FileCheck %s
; RUN: opt -instsimplify -S < %s 2>&1 -pass-remarks-analysis=.* | FileCheck %s
; Verify that warnings are emitted for the 2nd and 3rd tests.
; CHECK: remark: /tmp/s.c:1:13: Detected conflicting code assumptions.
; CHECK: remark: /tmp/s.c:4:10: Detected conflicting code assumptions.
define void @test1() {
; CHECK-LABEL: @test1(
@ -15,12 +20,12 @@ define void @test1() {
; return value. There's no way to win (we can't undo transforms that happened
; based on half-truths), so just don't crash.
define i64 @PR31809() {
define i64 @PR31809() !dbg !7 {
; CHECK-LABEL: @PR31809(
; CHECK-NEXT: ret i64 3
;
%a = alloca i32
%t1 = ptrtoint i32* %a to i64
%t1 = ptrtoint i32* %a to i64, !dbg !9
%cond = icmp eq i64 %t1, 3
call void @llvm.assume(i1 %cond)
ret i64 %t1
@ -30,14 +35,14 @@ define i64 @PR31809() {
; so just don't crash. The second icmp+assume gets processed later, so that
; determines the return value.
define i8 @conflicting_assumptions(i8 %x) {
define i8 @conflicting_assumptions(i8 %x) !dbg !10 {
; CHECK-LABEL: @conflicting_assumptions(
; CHECK-NEXT: call void @llvm.assume(i1 false)
; CHECK-NEXT: [[COND2:%.*]] = icmp eq i8 %x, 4
; CHECK-NEXT: call void @llvm.assume(i1 [[COND2]])
; CHECK-NEXT: ret i8 5
;
%add = add i8 %x, 1
%add = add i8 %x, 1, !dbg !11
%cond1 = icmp eq i8 %x, 3
call void @llvm.assume(i1 %cond1)
%cond2 = icmp eq i8 %x, 4
@ -47,3 +52,21 @@ define i8 @conflicting_assumptions(i8 %x) {
declare void @llvm.assume(i1) nounwind
!llvm.dbg.cu = !{!0}
!llvm.module.flags = !{!3, !4, !5}
!llvm.ident = !{!6}
!0 = distinct !DICompileUnit(language: DW_LANG_C99, file: !1, producer: "clang version 4.0.0 (trunk 282540) (llvm/trunk 282542)", isOptimized: true, runtimeVersion: 0, emissionKind: LineTablesOnly, enums: !2)
!1 = !DIFile(filename: "/tmp/s.c", directory: "/tmp")
!2 = !{}
!3 = !{i32 2, !"Dwarf Version", i32 4}
!4 = !{i32 2, !"Debug Info Version", i32 3}
!5 = !{i32 1, !"PIC Level", i32 2}
!6 = !{!"clang version 4.0.0 (trunk 282540) (llvm/trunk 282542)"}
!7 = distinct !DISubprogram(name: "foo", scope: !1, file: !1, line: 1, type: !8, isLocal: false, isDefinition: true, scopeLine: 1, isOptimized: true, unit: !0, variables: !2)
!8 = !DISubroutineType(types: !2)
!9 = !DILocation(line: 1, column: 13, scope: !7)
!10 = distinct !DISubprogram(name: "bar", scope: !1, file: !1, line: 3, type: !8, isLocal: false, isDefinition: true, scopeLine: 3, isOptimized: true, unit: !0, variables: !2)
!11 = !DILocation(line: 4, column: 10, scope: !10)
!12 = !DILocation(line: 4, column: 3, scope: !10)