[PATCH] D67205: [SimplifyCFG] Don't SimplifyBranchOnICmpChain with ExtraCase

[Vitaly Buka via Phabricator via llvm-commits]

vitalybuka created this revision.
vitalybuka added a reviewer: eugenis.
Herald added subscribers: llvm-commits, hiraditya.
Herald added a project: LLVM.

Sometimes newly created "explicit branch" check value which can be undef.
Before transformation, if source code was correct, undefs should not
reach any branch as they are not possible if earlier conditions satisfied.
However "explicit branch" is placed before the switch and may branch on undef.
See the test as example.

So far in the known case when that happens and undef reaches "explicit branch"
it's not important which way we branch there. Switch will select same
block as non-switch destination of "explicit branch". So I assume
transformation in general is correct.

However msan by design reports branches on uninitialized memory and
undefs, so we have false report here.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D67205

Files:
  llvm/lib/Transforms/Utils/SimplifyCFG.cpp
  llvm/test/Transforms/SimplifyCFG/switch_msan.ll


Index: llvm/test/Transforms/SimplifyCFG/switch_msan.ll
===================================================================
--- /dev/null
+++ llvm/test/Transforms/SimplifyCFG/switch_msan.ll
@@ -0,0 +1,75 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
+; RUN: opt -S -simplifycfg < %s | FileCheck %s
+
+declare i8 @next_char();
+
+define void @test_no_msan() {
+; CHECK-LABEL: define void @test_no_msan()
+
+entry:
+  br label %while.body
+
+while.body:
+  br label %while.body.i
+
+while.body.i:
+; CHECK-LABEL: while.body.i:
+  %maybe_undef = phi i1 [ undef, %while.body ], [ %next_maybe_undef, %while.body.i ]
+  %c = call fastcc signext i8 @next_char()
+  %c_10 = icmp eq i8 %c, 10
+  %c_13 = icmp eq i8 %c, 13
+  %c_10_or_13 = or i1 %c_10, %c_13
+  %next_maybe_undef = or i1 %maybe_undef, %c_10_or_13
+  %c_not_10_or_13 = xor i1 %c_10_or_13, true
+  br i1 %c_not_10_or_13, label %while.body.i, label %while.body.i.break
+  ; CHECK: br i1 %c_not_10_or_13, label %while.body.i, label %while.body.i.break
+
+while.body.i.break:
+; CHECK-LABEL: while.body.i.break:
+  ; NEXT_MAYBE_UNDEF is never undef if here
+  br i1 %next_maybe_undef, label %while.body, label %return
+  ; CHECK: br i1 %maybe_undef, label %while.body, label %switch.early.test
+
+; CHECK-LABEL: switch.early.test:
+; CHECK: switch i8 %c, label %return [
+; CHECK:   i8 13, label %while.body
+; CHECK:   i8 10, label %while.body
+; CHECK: ]
+
+return:
+  ret void
+}
+
+
+define void @test_msan() sanitize_memory {
+; CHECK-LABEL: define void @test_msan()
+
+entry:
+  br label %while.body
+
+while.body:
+  br label %while.body.i
+
+while.body.i:
+; CHECK-LABEL: while.body.i:
+  %maybe_undef = phi i1 [ undef, %while.body ], [ %next_maybe_undef, %while.body.i ]
+  %c = call fastcc signext i8 @next_char()
+  %c_10 = icmp eq i8 %c, 10
+  %c_13 = icmp eq i8 %c, 13
+  %c_10_or_13 = or i1 %c_10, %c_13
+  %next_maybe_undef = or i1 %maybe_undef, %c_10_or_13
+  %c_not_10_or_13 = xor i1 %c_10_or_13, true
+  br i1 %c_not_10_or_13, label %while.body.i, label %while.body.i.break
+  ; CHECK: br i1 %c_not_10_or_13, label %while.body.i, label %while.body.i.break
+
+while.body.i.break:
+; CHECK-LABEL: while.body.i.break:
+  ; NEXT_MAYBE_UNDEF is never undef if here
+  br i1 %next_maybe_undef, label %while.body, label %return
+  ; CHECK: br i1 %next_maybe_undef, label %while.body, label %return
+
+; CHECK-NOT: switch.early.test:
+
+return:
+  ret void
+}
Index: llvm/lib/Transforms/Utils/SimplifyCFG.cpp
===================================================================
--- llvm/lib/Transforms/Utils/SimplifyCFG.cpp
+++ llvm/lib/Transforms/Utils/SimplifyCFG.cpp
@@ -3734,9 +3734,13 @@
                     << *BB);
 
   // If there are any extra values that couldn't be folded into the switch
-  // then we evaluate them with an explicit branch first.  Split the block
+  // then we evaluate them with an explicit branch first. Split the block
   // right before the condbr to handle it.
   if (ExtraCase) {
+    // MSAN don't like undefs as branch condition which can be introduced
+    // with "explicit branch".
+    if (BB->getParent()->hasFnAttribute(Attribute::SanitizeMemory))
+      return false;
     BasicBlock *NewBB =
         BB->splitBasicBlock(BI->getIterator(), "switch.early.test");
     // Remove the uncond branch added to the old block.


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D67205.218823.patch
Type: text/x-patch
Size: 3357 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20190905/531ee1f5/attachment.bin>