[llvm] r294208 - [ValueTracking] emit a remark when we detect a conflicting assumption (PR31809)
Sanjay Patel via llvm-commits
llvm-commits at lists.llvm.org
Mon Feb 6 10:26:06 PST 2017
Author: spatel
Date: Mon Feb 6 12:26:06 2017
New Revision: 294208
URL: http://llvm.org/viewvc/llvm-project?rev=294208&view=rev
Log:
[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
Modified:
llvm/trunk/include/llvm/Analysis/InstructionSimplify.h
llvm/trunk/include/llvm/Analysis/ValueTracking.h
llvm/trunk/lib/Analysis/InstructionSimplify.cpp
llvm/trunk/lib/Analysis/ValueTracking.cpp
llvm/trunk/lib/Transforms/Utils/SimplifyInstructions.cpp
llvm/trunk/test/Transforms/InstSimplify/assume.ll
Modified: llvm/trunk/include/llvm/Analysis/InstructionSimplify.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Analysis/InstructionSimplify.h?rev=294208&r1=294207&r2=294208&view=diff
==============================================================================
--- llvm/trunk/include/llvm/Analysis/InstructionSimplify.h (original)
+++ llvm/trunk/include/llvm/Analysis/InstructionSimplify.h Mon Feb 6 12:26:06 2017
@@ -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.
///
Modified: llvm/trunk/include/llvm/Analysis/ValueTracking.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Analysis/ValueTracking.h?rev=294208&r1=294207&r2=294208&view=diff
==============================================================================
--- llvm/trunk/include/llvm/Analysis/ValueTracking.h (original)
+++ llvm/trunk/include/llvm/Analysis/ValueTracking.h Mon Feb 6 12:26:06 2017
@@ -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
Modified: llvm/trunk/lib/Analysis/InstructionSimplify.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Analysis/InstructionSimplify.cpp?rev=294208&r1=294207&r2=294208&view=diff
==============================================================================
--- llvm/trunk/lib/Analysis/InstructionSimplify.cpp (original)
+++ llvm/trunk/lib/Analysis/InstructionSimplify.cpp Mon Feb 6 12:26:06 2017
@@ -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, Arra
/// 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(Instruc
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);
}
Modified: llvm/trunk/lib/Analysis/ValueTracking.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Analysis/ValueTracking.cpp?rev=294208&r1=294207&r2=294208&view=diff
==============================================================================
--- llvm/trunk/lib/Analysis/ValueTracking.cpp (original)
+++ llvm/trunk/lib/Analysis/ValueTracking.cpp Mon Feb 6 12:26:06 2017
@@ -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
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(c
}
// 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.");
+ }
}
}
Modified: llvm/trunk/lib/Transforms/Utils/SimplifyInstructions.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Utils/SimplifyInstructions.cpp?rev=294208&r1=294207&r2=294208&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/Utils/SimplifyInstructions.cpp (original)
+++ llvm/trunk/lib/Transforms/Utils/SimplifyInstructions.cpp Mon Feb 6 12:26:06 2017
@@ -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 D
// 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, "i
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::ru
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();
Modified: llvm/trunk/test/Transforms/InstSimplify/assume.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/InstSimplify/assume.ll?rev=294208&r1=294207&r2=294208&view=diff
==============================================================================
--- llvm/trunk/test/Transforms/InstSimplify/assume.ll (original)
+++ llvm/trunk/test/Transforms/InstSimplify/assume.ll Mon Feb 6 12:26:06 2017
@@ -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)
+
More information about the llvm-commits
mailing list