[llvm] r339149 - [GVN,NewGVN] Keep nonnull if K does not move.

Florian Hahn via llvm-commits llvm-commits at lists.llvm.org
Tue Aug 7 08:36:11 PDT 2018


Author: fhahn
Date: Tue Aug  7 08:36:11 2018
New Revision: 339149

URL: http://llvm.org/viewvc/llvm-project?rev=339149&view=rev
Log:
[GVN,NewGVN] Keep nonnull if K does not move.

In combineMetadata, we should be able to preserve K's nonnull metadata,
if K does not move. This condition should hold for all replacements by
NewGVN/GVN, but I added a bunch of assertions to verify that.

Fixes PR35038.

There probably are additional kinds of metadata that could be preserved
using similar reasoning. This is follow-up work.

Reviewers: dberlin, davide, efriedma, nlopes

Reviewed By: efriedma

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

Added:
    llvm/trunk/test/Transforms/NewGVN/metadata-nonnull.ll
Modified:
    llvm/trunk/include/llvm/Transforms/Utils/Local.h
    llvm/trunk/lib/Transforms/Utils/Local.cpp

Modified: llvm/trunk/include/llvm/Transforms/Utils/Local.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Transforms/Utils/Local.h?rev=339149&r1=339148&r2=339149&view=diff
==============================================================================
--- llvm/trunk/include/llvm/Transforms/Utils/Local.h (original)
+++ llvm/trunk/include/llvm/Transforms/Utils/Local.h Tue Aug  7 08:36:11 2018
@@ -382,10 +382,13 @@ void removeUnwindEdge(BasicBlock *BB, Do
 bool removeUnreachableBlocks(Function &F, LazyValueInfo *LVI = nullptr,
                              DomTreeUpdater *DTU = nullptr);
 
-/// Combine the metadata of two instructions so that K can replace J
+/// Combine the metadata of two instructions so that K can replace J. Some
+/// metadata kinds can only be kept if K does not move, meaning it dominated
+/// J in the original IR.
 ///
 /// Metadata not listed as known via KnownIDs is removed
-void combineMetadata(Instruction *K, const Instruction *J, ArrayRef<unsigned> KnownIDs);
+void combineMetadata(Instruction *K, const Instruction *J,
+                     ArrayRef<unsigned> KnownIDs, bool DoesKMove = true);
 
 /// Combine the metadata of two instructions so that K can replace J. This
 /// specifically handles the case of CSE-like transformations.
@@ -394,7 +397,8 @@ void combineMetadata(Instruction *K, con
 void combineMetadataForCSE(Instruction *K, const Instruction *J);
 
 /// Patch the replacement so that it is not more restrictive than the value
-/// being replaced.
+/// being replaced. It assumes that the replacement does not get moved from
+/// its original position.
 void patchReplacementInstruction(Instruction *I, Value *Repl);
 
 // Replace each use of 'From' with 'To', if that use does not belong to basic

Modified: llvm/trunk/lib/Transforms/Utils/Local.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Utils/Local.cpp?rev=339149&r1=339148&r2=339149&view=diff
==============================================================================
--- llvm/trunk/lib/Transforms/Utils/Local.cpp (original)
+++ llvm/trunk/lib/Transforms/Utils/Local.cpp Tue Aug  7 08:36:11 2018
@@ -2279,7 +2279,7 @@ bool llvm::removeUnreachableBlocks(Funct
 }
 
 void llvm::combineMetadata(Instruction *K, const Instruction *J,
-                           ArrayRef<unsigned> KnownIDs) {
+                           ArrayRef<unsigned> KnownIDs, bool DoesKMove) {
   SmallVector<std::pair<unsigned, MDNode *>, 4> Metadata;
   K->dropUnknownNonDebugMetadata(KnownIDs);
   K->getAllMetadataOtherThanDebugLoc(Metadata);
@@ -2315,8 +2315,9 @@ void llvm::combineMetadata(Instruction *
         K->setMetadata(Kind, JMD);
         break;
       case LLVMContext::MD_nonnull:
-        // Only set the !nonnull if it is present in both instructions.
-        K->setMetadata(Kind, JMD);
+        // If K does move, keep nonull if it is present in both instructions.
+        if (DoesKMove)
+          K->setMetadata(Kind, JMD);
         break;
       case LLVMContext::MD_invariant_group:
         // Preserve !invariant.group in K.
@@ -2381,8 +2382,8 @@ void llvm::patchReplacementInstruction(I
       LLVMContext::MD_tbaa,            LLVMContext::MD_alias_scope,
       LLVMContext::MD_noalias,         LLVMContext::MD_range,
       LLVMContext::MD_fpmath,          LLVMContext::MD_invariant_load,
-      LLVMContext::MD_invariant_group};
-  combineMetadata(ReplInst, I, KnownIDs);
+      LLVMContext::MD_invariant_group, LLVMContext::MD_nonnull};
+  combineMetadata(ReplInst, I, KnownIDs, false);
 }
 
 template <typename RootType, typename DominatesFn>

Added: llvm/trunk/test/Transforms/NewGVN/metadata-nonnull.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/NewGVN/metadata-nonnull.ll?rev=339149&view=auto
==============================================================================
--- llvm/trunk/test/Transforms/NewGVN/metadata-nonnull.ll (added)
+++ llvm/trunk/test/Transforms/NewGVN/metadata-nonnull.ll Tue Aug  7 08:36:11 2018
@@ -0,0 +1,178 @@
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
+
+; RUN: opt %s -newgvn -S | FileCheck %s
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+define i8* @test1(i8** %v0, i8** %v1) {
+; CHECK-LABEL: @test1(
+; CHECK-NEXT:  top:
+; CHECK-NEXT:    [[V2:%.*]] = load i8*, i8** [[V0:%[a-z0-9]+]], !nonnull !0
+; CHECK-NEXT:    store i8* [[V2]], i8** [[V1:%.*]]
+; CHECK-NEXT:    ret i8* [[V2]]
+;
+top:
+  %v2 = load i8*, i8** %v0, !nonnull !0
+  store i8* %v2, i8** %v1
+  %v3 = load i8*, i8** %v1
+  ret i8* %v3
+}
+
+; FIXME: could propagate nonnull to first load?
+define i8* @test2(i8** %v0, i8** %v1) {
+; CHECK-LABEL: @test2(
+; CHECK-NEXT:  top:
+; CHECK-NEXT:    [[V2:%.*]] = load i8*, i8** [[V0:%[a-z0-9]+]]
+; CHECK-NOT:     !nonnull
+; CHECK-NEXT:    store i8* [[V2]], i8** [[V1:%.*]]
+; CHECK-NEXT:    ret i8* [[V2]]
+;
+top:
+  %v2 = load i8*, i8** %v0
+  store i8* %v2, i8** %v1
+  %v3 = load i8*, i8** %v1, !nonnull !0
+  ret i8* %v3
+}
+
+declare void @use1(i8* %a) readonly
+
+define i8* @test3(i8** %v0) {
+; CHECK-LABEL: @test3(
+; CHECK-NEXT:  top:
+; CHECK-NEXT:    [[V1:%.*]] = load i8*, i8** [[V0:%[a-z0-9]+]]
+; CHECK-NOT:     !nonnull
+; CHECK-NEXT:    call void @use1(i8* [[V1]])
+; CHECK-NEXT:    br i1 undef, label [[BB1:%.*]], label [[BB2:%.*]]
+; CHECK:       bb1:
+; CHECK-NEXT:    ret i8* [[V1]]
+; CHECK:       bb2:
+; CHECK-NEXT:    ret i8* [[V1]]
+;
+top:
+  %v1 = load i8*, i8** %v0
+  call void @use1(i8* %v1)
+  br i1 undef, label %bb1, label %bb2
+
+bb1:
+  %v2 = load i8*, i8** %v0, !nonnull !0
+  ret i8* %v2
+
+bb2:
+  %v3 = load i8*, i8** %v0
+  ret i8* %v3
+}
+
+define i8* @test4(i8** %v0) {
+; CHECK-LABEL: @test4(
+; CHECK-NEXT:  top:
+; CHECK-NEXT:    [[V1:%.*]] = load i8*, i8** [[V0:%[a-z0-9]+]]
+; CHECK-NOT:     !nonnull
+; CHECK-NEXT:    call void @use1(i8* [[V1]])
+; CHECK-NEXT:    br i1 undef, label [[BB1:%.*]], label [[BB2:%.*]]
+; CHECK:       bb1:
+; CHECK-NEXT:    ret i8* [[V1]]
+; CHECK:       bb2:
+; CHECK-NEXT:    ret i8* [[V1]]
+;
+top:
+  %v1 = load i8*, i8** %v0
+  call void @use1(i8* %v1)
+  br i1 undef, label %bb1, label %bb2
+
+bb1:
+  %v2 = load i8*, i8** %v0
+  ret i8* %v2
+
+bb2:
+  %v3 = load i8*, i8** %v0, !nonnull !0
+  ret i8* %v3
+}
+
+define i8* @test5(i8** %v0) {
+; CHECK-LABEL: @test5(
+; CHECK-NEXT:  top:
+; CHECK-NEXT:    [[V1:%.*]] = load i8*, i8** [[V0:%[a-z0-9]+]], !nonnull !0
+; CHECK-NEXT:    call void @use1(i8* [[V1]])
+; CHECK-NEXT:    br i1 undef, label [[BB1:%.*]], label [[BB2:%.*]]
+; CHECK:       bb1:
+; CHECK-NEXT:    ret i8* [[V1]]
+; CHECK:       bb2:
+; CHECK-NEXT:    ret i8* [[V1]]
+;
+top:
+  %v1 = load i8*, i8** %v0, !nonnull !0
+  call void @use1(i8* %v1)
+  br i1 undef, label %bb1, label %bb2
+
+bb1:
+  %v2 = load i8*, i8** %v0
+  ret i8* %v2
+
+bb2:
+  %v3 = load i8*, i8** %v0
+  ret i8* %v3
+}
+
+define i8* @test6(i8** %v0, i8** %v1) {
+; CHECK-LABEL: @test6(
+; CHECK-NEXT:  top:
+; CHECK-NEXT:    br i1 undef, label [[BB1:%.*]], label [[BB2:%.*]]
+; CHECK:       bb1:
+; CHECK-NEXT:    [[V2:%.*]] = load i8*, i8** [[V0:%[a-z0-9]+]], !nonnull !0
+; CHECK-NEXT:    store i8* [[V2]], i8** [[V1:%.*]]
+; CHECK-NEXT:    ret i8* [[V2]]
+; CHECK:       bb2:
+; CHECK-NEXT:    [[V4:%.*]] = load i8*, i8** [[V0]]
+; CHECK-NOT:     !nonnull
+; CHECK-NEXT:    store i8* [[V4]], i8** [[V1]]
+; CHECK-NOT:     !nonnull
+; CHECK-NEXT:    ret i8* [[V4]]
+;
+top:
+  br i1 undef, label %bb1, label %bb2
+
+bb1:
+  %v2 = load i8*, i8** %v0, !nonnull !0
+  store i8* %v2, i8** %v1
+  %v3 = load i8*, i8** %v1
+  ret i8* %v3
+
+bb2:
+  %v4 = load i8*, i8** %v0
+  store i8* %v4, i8** %v1
+  %v5 = load i8*, i8** %v1, !nonnull !0
+  ret i8* %v5
+}
+
+declare void @use2(i8* %a)
+
+define i8* @test7(i8** %v0) {
+; CHECK-LABEL: @test7(
+; CHECK-NEXT:  top:
+; CHECK-NEXT:    [[V1:%.*]] = load i8*, i8** [[V0:%[a-z0-9]+]], !nonnull !0
+; CHECK-NEXT:    call void @use2(i8* [[V1]])
+; CHECK-NEXT:    br i1 undef, label [[BB1:%.*]], label [[BB2:%.*]]
+; CHECK:       bb1:
+; CHECK-NEXT:    [[V2:%.*]] = load i8*, i8** [[V0]]
+; CHECK-NOT:     !nonnull
+; CHECK-NEXT:    ret i8* [[V2]]
+; CHECK:       bb2:
+; CHECK-NEXT:    [[V3:%.*]] = load i8*, i8** [[V0]]
+; CHECK-NOT:     !nonnull
+; CHECK-NEXT:    ret i8* [[V3]]
+;
+top:
+  %v1 = load i8*, i8** %v0, !nonnull !0
+  call void @use2(i8* %v1)
+  br i1 undef, label %bb1, label %bb2
+
+bb1:
+  %v2 = load i8*, i8** %v0
+  ret i8* %v2
+
+bb2:
+  %v3 = load i8*, i8** %v0
+  ret i8* %v3
+}
+
+!0 = !{}




More information about the llvm-commits mailing list