[llvm-reduce] Try harder to not create invalid aliases

This was done by adding --abort-on-invalid-reduction to remove-function-bodies-used-in-globals.ll and fixing the fallout. Aliases must have a GlobalValue or ConstantExpr aliasee and the aliasee must be a definition if it's a GlobalValue. Don't RAUW functions with null if there's an alias pointing to it, and similarly don't delete the body of a function. Don't delete the entire body of a function when reducing blocks, preserve at least one block. Also make debugging these sorts of things easier by dumping the module when --abort-on-invalid-reduction triggers. Reviewed By: regehr Differential Revision: https://reviews.llvm.org/D131505
intel · Aug 12, 2022 · 195087d · 195087d
1 parent bd1f80f
commit 195087d
Show file tree

Hide file tree

Showing 11 changed files with 45 additions and 61 deletions.
diff --git a/llvm/test/tools/llvm-reduce/remove-bbs-comdat.ll b/llvm/test/tools/llvm-reduce/remove-bbs-comdat.ll
diff --git a/llvm/test/tools/llvm-reduce/remove-bbs-entry.ll b/llvm/test/tools/llvm-reduce/remove-bbs-entry.ll
@@ -1,18 +1,14 @@
-; Test that llvm-reduce correctly removes the entry block of functions for
-; linkages other than external and weak.
+; Test that llvm-reduce does not remove the entry block of functions.
 ;
 ; RUN: llvm-reduce --abort-on-invalid-reduction --delta-passes=basic-blocks --test FileCheck --test-arg --check-prefixes=CHECK-INTERESTINGNESS --test-arg %s --test-arg --input-file %s -o %t
 ; RUN: cat %t | FileCheck %s
 
-; CHECK-INTERESTINGNESS: interesting1:
+; CHECK-INTERESTINGNESS: foo
 
-; CHECK-NOT: uninteresting
-define linkonce_odr i32 @foo() {
+; CHECK: add i32
+define i32 @foo() {
 uninteresting:
+  %a = add i32 0, 0
   ret i32 0
 }
 
-define i32 @main(i1 %c) {
-interesting1:
-  ret i32 0
-}
diff --git a/llvm/test/tools/llvm-reduce/remove-function-bodies-used-in-globals.ll b/llvm/test/tools/llvm-reduce/remove-function-bodies-used-in-globals.ll
@@ -1,4 +1,4 @@
-; RUN: llvm-reduce --test FileCheck --test-arg --check-prefix=CHECK-INTERESTINGNESS --test-arg %s --test-arg --input-file %s -o %t
+; RUN: llvm-reduce --test FileCheck --test-arg --check-prefix=CHECK-INTERESTINGNESS --abort-on-invalid-reduction --test-arg %s --test-arg --input-file %s -o %t
 ; RUN: FileCheck --check-prefix=CHECK-FINAL %s < %t
 
 ; We cannot change the @alias to undef, because it would result in invalid IR

diff --git a/llvm/test/tools/llvm-reduce/temporary-files-as-bitcode-split.ll b/llvm/test/tools/llvm-reduce/temporary-files-as-bitcode-split.ll
@@ -1,5 +1,5 @@
 ; RUN: opt --thinlto-bc --thinlto-split-lto-unit %s -o %t0
-; RUN: llvm-reduce -write-tmp-files-as-bitcode --delta-passes=basic-blocks %t0 -o %t1 \
+; RUN: llvm-reduce -write-tmp-files-as-bitcode --delta-passes=function-bodies,basic-blocks %t0 -o %t1 \
 ; RUN:     --test %python --test-arg %p/Inputs/llvm-dis-and-filecheck.py --test-arg llvm-dis --test-arg FileCheck --test-arg --check-prefixes=CHECK-ALL,CHECK-INTERESTINGNESS --test-arg %s
 ; RUN: cat %t1* | FileCheck --check-prefixes=CHECK-ALL,CHECK-FINAL %s
 

diff --git a/llvm/test/tools/llvm-reduce/temporary-files-as-bitcode.ll b/llvm/test/tools/llvm-reduce/temporary-files-as-bitcode.ll
@@ -1,4 +1,4 @@
-; RUN: llvm-reduce -write-tmp-files-as-bitcode --delta-passes=basic-blocks %s -o %t \
+; RUN: llvm-reduce -write-tmp-files-as-bitcode --delta-passes=function-bodies,basic-blocks %s -o %t \
 ; RUN:     --test %python --test-arg %p/Inputs/llvm-dis-and-filecheck.py --test-arg llvm-dis --test-arg FileCheck --test-arg --check-prefixes=CHECK-ALL,CHECK-INTERESTINGNESS --test-arg %s
 ; RUN: cat %t | FileCheck --check-prefixes=CHECK-ALL,CHECK-FINAL %s
 

diff --git a/llvm/tools/llvm-reduce/deltas/Delta.cpp b/llvm/tools/llvm-reduce/deltas/Delta.cpp
@@ -173,7 +173,8 @@ CheckChunk(Chunk &ChunkToCheckForUninterestingness,
   // Some reductions may result in invalid IR. Skip such reductions.
   if (verifyReducerWorkItem(*Clone, &errs())) {
     if (AbortOnInvalidReduction) {
-      errs() << "Invalid reduction\n";
+      errs() << "Invalid reduction, aborting.\n";
+      Clone->print(errs());
       exit(1);
     }
     errs() << " **** WARNING | reduction resulted in invalid module, "

diff --git a/llvm/tools/llvm-reduce/deltas/ReduceBasicBlocks.cpp b/llvm/tools/llvm-reduce/deltas/ReduceBasicBlocks.cpp
@@ -104,25 +104,32 @@ removeUninterestingBBsFromSwitch(SwitchInst &SwInst,
 
 /// It's OK to add a block to the set of removed blocks if the first
 /// basic block in the function that survives all of the deletions is
-/// a legal entry block
+/// a legal entry block. Keep at least one block in a function.
 static bool okToRemove(BasicBlock &Candidate, Function &F,
                        const DenseSet<BasicBlock *> &BBsToDelete) {
+  size_t NumBlocksDeleted = 0;
+  bool FoundNewEntryBlock = false;
   for (auto &B : F) {
     if (&B == &Candidate)
       continue;
-    if (BBsToDelete.count(&B))
+    if (BBsToDelete.count(&B)) {
+      ++NumBlocksDeleted;
       continue;
-    /// Ok we've found the first block that's not going to be deleted,
-    /// it will be the new entry block -- that's only legal if this
-    /// block has no predecessors among blocks that survive the
-    /// deletions
-    for (BasicBlock *Pred : predecessors(&B)) {
-      if (!BBsToDelete.contains(Pred))
-        return false;
     }
-    return true;
+    if (!FoundNewEntryBlock) {
+      /// Ok we've found the first block that's not going to be deleted,
+      /// it will be the new entry block -- that's only legal if this
+      /// block has no predecessors among blocks that survive the
+      /// deletions
+      for (BasicBlock *Pred : predecessors(&B)) {
+        if (!BBsToDelete.contains(Pred))
+          return false;
+      }
+      FoundNewEntryBlock = true;
+    }
   }
-  return true;
+  // Don't delete the last block.
+  return NumBlocksDeleted + 1 < F.size();
 }
 
 /// Removes out-of-chunk arguments from functions, and modifies their calls
@@ -161,19 +168,13 @@ static void extractBasicBlocksFromModule(Oracle &O, Module &Program) {
     // Instructions might be referenced in other BBs
     for (auto &I : *BB)
       I.replaceAllUsesWith(getDefaultValue(I.getType()));
-    if (BB->getParent()->size() == 1) {
-      // this is the last basic block of the function, thus we must also make
-      // sure to remove comdat and set linkage to external
-      auto F = BB->getParent();
-      F->deleteBody();
-      F->setComdat(nullptr);
-    } else {
-      BB->eraseFromParent();
-    }
+    // Should not be completely removing the body of a function.
+    assert(BB->getParent()->size() > 1);
+    BB->eraseFromParent();
   }
 }
 
 void llvm::reduceBasicBlocksDeltaPass(TestRunner &Test) {
-  outs() << "*** Reducing Basic Blocks...\n";
+  errs() << "*** Reducing Basic Blocks...\n";
   runDeltaPass(Test, extractBasicBlocksFromModule);
 }
diff --git a/llvm/tools/llvm-reduce/deltas/ReduceFunctionBodies.cpp b/llvm/tools/llvm-reduce/deltas/ReduceFunctionBodies.cpp
@@ -13,6 +13,7 @@
 
 #include "ReduceFunctionBodies.h"
 #include "Delta.h"
+#include "Utils.h"
 #include "llvm/IR/GlobalValue.h"
 
 using namespace llvm;
@@ -21,12 +22,12 @@ using namespace llvm;
 /// desired Chunks.
 static void extractFunctionBodiesFromModule(Oracle &O, Module &Program) {
   // Delete out-of-chunk function bodies
-  std::vector<Function *> FuncDefsToReduce;
-  for (auto &F : Program)
-    if (!F.isDeclaration() && !O.shouldKeep()) {
+  for (auto &F : Program) {
+    if (!F.isDeclaration() && !hasAliasUse(F) && !O.shouldKeep()) {
       F.deleteBody();
       F.setComdat(nullptr);
     }
+  }
 }
 
 void llvm::reduceFunctionBodiesDeltaPass(TestRunner &Test) {

diff --git a/llvm/tools/llvm-reduce/deltas/ReduceFunctions.cpp b/llvm/tools/llvm-reduce/deltas/ReduceFunctions.cpp
@@ -16,8 +16,6 @@
 #include "Delta.h"
 #include "Utils.h"
 #include "llvm/ADT/STLExtras.h"
-#include "llvm/IR/Constants.h"
-#include "llvm/IR/Instructions.h"
 #include <iterator>
 #include <vector>
 
@@ -33,7 +31,8 @@ static void extractFunctionsFromModule(Oracle &O, Module &Program) {
             // Intrinsics don't have function bodies that are useful to
             // reduce. Additionally, intrinsics may have additional operand
             // constraints. But, do drop intrinsics that are not referenced.
-            return (!F.isIntrinsic() || F.use_empty()) && !O.shouldKeep();
+            return (!F.isIntrinsic() || F.use_empty()) && !hasAliasUse(F) &&
+                   !O.shouldKeep();
           });
 
   // Then, drop body of each of them. We want to batch this and do nothing else

diff --git a/llvm/tools/llvm-reduce/deltas/Utils.cpp b/llvm/tools/llvm-reduce/deltas/Utils.cpp
@@ -12,9 +12,14 @@
 
 #include "Utils.h"
 #include "llvm/IR/Constants.h"
+#include "llvm/IR/GlobalAlias.h"
 
 using namespace llvm;
 
 Value *llvm::getDefaultValue(Type *T) {
   return T->isVoidTy() ? PoisonValue::get(T) : Constant::getNullValue(T);
 }
+
+bool llvm::hasAliasUse(Function &F) {
+  return any_of(F.users(), [](User *U) { return isa<GlobalAlias>(U); });
+}
diff --git a/llvm/tools/llvm-reduce/deltas/Utils.h b/llvm/tools/llvm-reduce/deltas/Utils.h
@@ -13,11 +13,13 @@
 #ifndef LLVM_TOOLS_LLVM_REDUCE_DELTAS_UTILS_H
 #define LLVM_TOOLS_LLVM_REDUCE_DELTAS_UTILS_H
 
+#include "llvm/IR/Function.h"
 #include "llvm/IR/Value.h"
 
 namespace llvm {
 
 Value *getDefaultValue(Type *T);
+bool hasAliasUse(Function &F);
 
 } // namespace llvm