DanilM created this revision.
DanilM added reviewers: hfinkel, rjmccall, kosarev.
Herald added a subscriber: cfe-commits.

If SimplifyCFG attaches one block to another it removes all metadata from 
instructions of a block that should be attached because it can be depended on 
conditions that are changed during transformation.
In case of TBAA metadata, it looks too conservatively: data types of load/store 
instructions cannot be changed during such transformations even if the pointed 
data was changed.
So attached the patch saves TBAA tags from removing during SimplifyCFG 
transformations.


Repository:
  rC Clang

https://reviews.llvm.org/D44617

Files:
  lib/Transforms/Utils/SimplifyCFG.cpp
  test/Transforms/SimplifyCFG/basictest.ll
  test/Transforms/SimplifyCFG/fold-two-entry-phi-tbaa.ll
  test/Transforms/SimplifyCFG/speculate-tbaa.ll

Index: test/Transforms/SimplifyCFG/speculate-tbaa.ll
===================================================================
--- test/Transforms/SimplifyCFG/speculate-tbaa.ll
+++ test/Transforms/SimplifyCFG/speculate-tbaa.ll
@@ -0,0 +1,53 @@
+; RUN: opt < %s -S -simplifycfg | FileCheck %s
+
+; This test case was generated from speculate-tbaa.c:
+;
+; void foo(int *p1, int p2, int condition) {
+;   *p1 = 100;
+;   if (condition)
+;     *p1 = p2;
+; }
+;
+; using
+;   clang -cc1 -triple x86_64-linux-gnu -new-struct-path-tbaa -emit-llvm -o - -O3 -disable-llvm-passes speculate-tbaa.c | opt -mem2reg -S
+
+; ModuleID = '<stdin>'
+source_filename = "speculate-tbaa.c"
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64--linux-gnu"
+
+; Function Attrs: nounwind
+define void @foo(i32* %p1, i32 %p2, i32 %condition) #0 {
+; CHECK-LABEL: @foo(
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:    store i32 100, i32* %p1, align 4, !tbaa [[TBAA_Tag:![0-9]+]]
+; CHECK-NEXT:    %tobool = icmp ne i32 %condition, 0
+; CHECK-NEXT:    %spec.store.select = select i1 %tobool, i32 %p2, i32 100
+; CHECK-NEXT:    store i32 %spec.store.select, i32* %p1, align 4, !tbaa [[TBAA_Tag]]
+entry:
+  store i32 100, i32* %p1, align 4, !tbaa !2
+  %tobool = icmp ne i32 %condition, 0
+  br i1 %tobool, label %if.then, label %if.end
+
+if.then:                                          ; preds = %entry
+  store i32 %p2, i32* %p1, align 4, !tbaa !2
+  br label %if.end
+
+if.end:                                           ; preds = %if.then, %entry
+  ret void
+}
+
+attributes #0 = { nounwind "correctly-rounded-divide-sqrt-fp-math"="false" "disable-tail-calls"="false" "less-precise-fpmad"="false" "no-frame-pointer-elim"="false" "no-infs-fp-math"="false" "no-jump-tables"="false" "no-nans-fp-math"="false" "no-signed-zeros-fp-math"="false" "no-trapping-math"="false" "stack-protector-buffer-size"="8" "target-features"="+mmx,+sse,+sse2,+x87" "unsafe-fp-math"="false" "use-soft-float"="false" }
+
+!llvm.module.flags = !{!0}
+!llvm.ident = !{!1}
+
+; CHECK-DAG: [[TBAA_Tag]] = !{[[TBAA_Type:![0-9]+]], [[TBAA_Type]], i64 0, i64 4}
+; CHECK-DAG: [[TBAA_Type]] = !{!{{[0-9]+}}, i64 4, !"int"}
+!0 = !{i32 1, !"wchar_size", i32 4}
+!1 = !{!"clang version 7.0.0 (trunk 326065)"}
+!2 = !{!3, !3, i64 0, i64 4}
+!3 = !{!4, i64 4, !"int"}
+!4 = !{!5, i64 1, !"omnipotent char"}
+!5 = !{!"Simple C/C++ TBAA"}
+
Index: test/Transforms/SimplifyCFG/fold-two-entry-phi-tbaa.ll
===================================================================
--- test/Transforms/SimplifyCFG/fold-two-entry-phi-tbaa.ll
+++ test/Transforms/SimplifyCFG/fold-two-entry-phi-tbaa.ll
@@ -0,0 +1,75 @@
+; RUN: opt < %s -S -simplifycfg | FileCheck %s
+
+; This test case was generated from fold-two-entry-phi-tbaa.c:
+;
+; int foo(int p1, int p2, int p3) {
+;   return p1 ? p1 : p2 ? p2 : p3;
+; }
+;
+; using
+;   clang -cc1 -triple x86_64-linux-gnu -new-struct-path-tbaa -emit-llvm -o - -O3 -disable-llvm-passes fold-two-entry-phi-tbaa.c
+
+; ModuleID = 'fold-two-entry-phi-tbaa.c'
+source_filename = "fold-two-entry-phi-tbaa.c"
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64--linux-gnu"
+
+; Function Attrs: nounwind
+define i32 @foo(i32 %p1, i32 %p2, i32 %p3) #0 {
+entry:
+  %p1.addr = alloca i32, align 4
+  %p2.addr = alloca i32, align 4
+  %p3.addr = alloca i32, align 4
+  store i32 %p1, i32* %p1.addr, align 4, !tbaa !2
+  store i32 %p2, i32* %p2.addr, align 4, !tbaa !2
+  store i32 %p3, i32* %p3.addr, align 4, !tbaa !2
+  %0 = load i32, i32* %p1.addr, align 4, !tbaa !2
+  %tobool = icmp ne i32 %0, 0
+  br i1 %tobool, label %cond.true, label %cond.false
+
+cond.true:                                        ; preds = %entry
+  %1 = load i32, i32* %p1.addr, align 4, !tbaa !2
+  br label %cond.end4
+
+; CHECK: cond.false:
+; CHECK-NEXT:  %2 = load i32, i32* %p2.addr, align 4, !tbaa [[TBAA_Tag:![0-9]+]]
+; CHECK-NEXT:  %tobool1 = icmp ne i32 %2, 0
+; CHECK-NEXT:  %3 = load i32, i32* %p2.addr, align 4, !tbaa [[TBAA_Tag]]
+; CHECK-NEXT:  %4 = load i32, i32* %p3.addr, align 4, !tbaa [[TBAA_Tag]]
+; CHECK-NEXT:  %cond = select i1 %tobool1, i32 %3, i32 %4
+; CHECK-NEXT:  br label %cond.end4
+cond.false:                                       ; preds = %entry
+  %2 = load i32, i32* %p2.addr, align 4, !tbaa !2
+  %tobool1 = icmp ne i32 %2, 0
+  br i1 %tobool1, label %cond.true2, label %cond.false3
+
+cond.true2:                                       ; preds = %cond.false
+  %3 = load i32, i32* %p2.addr, align 4, !tbaa !2
+  br label %cond.end
+
+cond.false3:                                      ; preds = %cond.false
+  %4 = load i32, i32* %p3.addr, align 4, !tbaa !2
+  br label %cond.end
+
+cond.end:                                         ; preds = %cond.false3, %cond.true2
+  %cond = phi i32 [ %3, %cond.true2 ], [ %4, %cond.false3 ]
+  br label %cond.end4
+
+cond.end4:                                        ; preds = %cond.end, %cond.true
+  %cond5 = phi i32 [ %1, %cond.true ], [ %cond, %cond.end ]
+  ret i32 %cond5
+}
+
+attributes #0 = { nounwind "correctly-rounded-divide-sqrt-fp-math"="false" "disable-tail-calls"="false" "less-precise-fpmad"="false" "no-frame-pointer-elim"="false" "no-infs-fp-math"="false" "no-jump-tables"="false" "no-nans-fp-math"="false" "no-signed-zeros-fp-math"="false" "no-trapping-math"="false" "stack-protector-buffer-size"="8" "target-features"="+mmx,+sse,+sse2,+x87" "unsafe-fp-math"="false" "use-soft-float"="false" }
+
+!llvm.module.flags = !{!0}
+!llvm.ident = !{!1}
+
+; CHECK-DAG: [[TBAA_Tag]] = !{[[TBAA_Type:![0-9]+]], [[TBAA_Type]], i64 0, i64 4}
+; CHECK-DAG: [[TBAA_Type]] = !{!{{[0-9]+}}, i64 4, !"int"}
+!0 = !{i32 1, !"wchar_size", i32 4}
+!1 = !{!"clang version 7.0.0 (trunk 326065)"}
+!2 = !{!3, !3, i64 0, i64 4}
+!3 = !{!4, i64 4, !"int"}
+!4 = !{!5, i64 1, !"omnipotent char"}
+!5 = !{!"Simple C/C++ TBAA"}
Index: test/Transforms/SimplifyCFG/basictest.ll
===================================================================
--- test/Transforms/SimplifyCFG/basictest.ll
+++ test/Transforms/SimplifyCFG/basictest.ll
@@ -98,15 +98,15 @@
 ; CHECK: alloca i8, align 1
 ; CHECK-NEXT: call i8 @test6g
 ; CHECK-NEXT: icmp eq i8 %tmp, 0
-; CHECK-NEXT: load i8, i8* %r, align 1, !dbg !{{[0-9]+$}}
+; CHECK-NEXT: load i8, i8* %r, align 1, !dbg !{{[0-9]+}}, !tbaa [[TBAA_Tag:![0-9]+]]
 
 bb0:
   %r = alloca i8, align 1
   %tmp = call i8 @test6g(i8* %r)
   %tmp1 = icmp eq i8 %tmp, 0
   br i1 %tmp1, label %bb2, label %bb1
 bb1:
-  %tmp3 = load i8, i8* %r, align 1, !range !2, !tbaa !10, !dbg !5
+  %tmp3 = load i8, i8* %r, align 1, !range !2, !tbaa !0, !dbg !5
   %tmp4 = icmp eq i8 %tmp3, 1
   br i1 %tmp4, label %bb2, label %bb3
 bb2:
@@ -120,14 +120,15 @@
 !llvm.dbg.cu = !{!3}
 !llvm.module.flags = !{!8, !9}
 
-!0 = !{!10, !10, i64 0}
-!1 = !{!"foo"}
+; CHECK-DAG: [[TBAA_Tag]] = !{[[TBAA_Type:![0-9]+]], [[TBAA_Type]], i64 0, i64 1}
+!0 = !{!10, !10, i64 0, i64 1}
+!1 = !{!"TBAA root"}
 !2 = !{i8 0, i8 2}
 !3 = distinct !DICompileUnit(language: DW_LANG_C99, file: !7, producer: "clang", isOptimized: false, runtimeVersion: 0, emissionKind: FullDebug, enums: !4, globals: !4)
 !4 = !{}
 !5 = !DILocation(line: 23, scope: !6)
 !6 = distinct !DISubprogram(name: "foo", scope: !3, file: !7, line: 1, type: !DISubroutineType(types: !4), isLocal: false, isDefinition: true, scopeLine: 1, flags: DIFlagPrototyped, isOptimized: false, unit: !3, variables: !4)
 !7 = !DIFile(filename: "foo.c", directory: "/")
 !8 = !{i32 2, !"Dwarf Version", i32 2}
 !9 = !{i32 2, !"Debug Info Version", i32 3}
-!10 = !{!"scalar type", !1}
+!10 = !{!1, i64 1, !"scalar type"}
Index: lib/Transforms/Utils/SimplifyCFG.cpp
===================================================================
--- lib/Transforms/Utils/SimplifyCFG.cpp
+++ lib/Transforms/Utils/SimplifyCFG.cpp
@@ -153,6 +153,11 @@
 // switch for that PHI.
 using SwitchCaseResultsTy = SmallVector<std::pair<PHINode *, Constant *>, 4>;
 
+// When SimplifyCFG attaches one block to another, it removes metadata from
+// instructions that will be attached. This array contains metadata names that
+// should be remained.
+const unsigned ValidMetadataAfterBlockMerging[] = { LLVMContext::MD_tbaa };
+
 /// ValueEqualityComparisonCase - Represents a case of a switch.
 struct ValueEqualityComparisonCase {
   ConstantInt *Value;
@@ -2077,8 +2082,8 @@
   // Metadata can be dependent on the condition we are hoisting above.
   // Conservatively strip all metadata on the instruction.
   for (auto &I : *ThenBB)
-    I.dropUnknownNonDebugMetadata();
-
+    I.dropUnknownNonDebugMetadata(ValidMetadataAfterBlockMerging);
+  
   // Hoist the instructions.
   BB->getInstList().splice(BI->getIterator(), ThenBB->getInstList(),
                            ThenBB->begin(), std::prev(ThenBB->end()));
@@ -2375,14 +2380,14 @@
   // conditional parts of the if's up to the dominating block.
   if (IfBlock1) {
     for (auto &I : *IfBlock1)
-      I.dropUnknownNonDebugMetadata();
+      I.dropUnknownNonDebugMetadata(ValidMetadataAfterBlockMerging);
     DomBlock->getInstList().splice(InsertPt->getIterator(),
                                    IfBlock1->getInstList(), IfBlock1->begin(),
                                    IfBlock1->getTerminator()->getIterator());
   }
   if (IfBlock2) {
     for (auto &I : *IfBlock2)
-      I.dropUnknownNonDebugMetadata();
+      I.dropUnknownNonDebugMetadata(ValidMetadataAfterBlockMerging);
     DomBlock->getInstList().splice(InsertPt->getIterator(),
                                    IfBlock2->getInstList(), IfBlock2->begin(),
                                    IfBlock2->getTerminator()->getIterator());
@@ -2702,7 +2707,7 @@
       // only given the branch precondition.
       // For an analogous reason, we must also drop all the metadata whose
       // semantics we don't understand.
-      NewBonusInst->dropUnknownNonDebugMetadata();
+      NewBonusInst->dropUnknownNonDebugMetadata(ValidMetadataAfterBlockMerging);
 
       PredBlock->getInstList().insert(PBI->getIterator(), NewBonusInst);
       NewBonusInst->takeName(&*BonusInst);
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
  • [PATCH] D44617: [Transforms... Danil Malyshev via Phabricator via cfe-commits

Reply via email to