[PATCH] D19211: Teach ValueTracking that compare-and-branch on poison is UB

Sanjoy Das via llvm-commits llvm-commits at lists.llvm.org
Mon Apr 18 00:07:21 PDT 2016


sanjoy added inline comments.

================
Comment at: lib/Analysis/ValueTracking.cpp:3413-3416
@@ -3407,1 +3412,6 @@
 
+    case Instruction::Br:
+      if (cast<BranchInst>(I)->isConditional())
+        return cast<BranchInst>(I)->getCondition();
+      return nullptr;
+
----------------
majnemer wrote:
> Our langref states:
> > An instruction control-depends on a terminator instruction if the terminator instruction has multiple successors and the instruction is always executed when control transfers to one of the successors, and may not be executed when control is transferred to another.
> 
> The code, as written, sounds like it will do the wrong thing if the branch instruction branches to the same basic block regardless of the condition operand.  Maybe this is just a bug in the langref?  Equally possible is that I misunderstand the behavior that this function is trying to suss out.
You're right; as written this code is not correct and it can't easily be fixed either.  Going by the langref, this code fragment (the motivation for this change)

```
loop:
  %iv = phi(0, %iv.inc)
  %iv.inc = add nsw %iv, 1
  %cmp = %iv.inc s< %L
  br i1 %cmp, label %leave, label %loop
leave:
  ...
```

is well defined, even when `%iv.inc` is poison.

/sadtrombone


http://reviews.llvm.org/D19211





More information about the llvm-commits mailing list