llvm-reduce: Fix assertion on blockaddress during function reduction

[arsenm]

Summary

Just avoid crashing for now, we should be able to replace the blockaddresses
themselves.

BlockAddress::handleOperandChangeImpl assumes it can cast to Function.
The verifier seems nonexistent and the langref isn't particularly explicit
on what's allowed as a blockaddress operand. As far as I can tell bugpoint
isn't doing anything to handle this.

Something low level is broken with BlockAddress handling,
demonstrated by reduce-functions-blockaddress-wrong-function.ll.
The BasicBlock destructor of the deleted function is triggering replacement
of blockaddresses for the kept function in some cases. I've only half debugged
this but it seems like blockaddress is handled too-specially compared to other
Constants. I have tentative patches to allow any constant to be a blockaddress
input, but having the verifier check if it's really a function/block.

[arsenm]

Use separate file for addrspacecasted case. Something horrible seems to be happening if there are actual uses of the addrspacecasted version in the same file

[arsenm]

Re-merge test. The weird behavior shows up with any other unused function present, I think because the removal logic special cases unused functions

[arsenm]

Add a testcase showing wrong blockaddress replacement if a different function is deleted. I don't think this is a proper llvm-reduce bug and something seems broadly wrong with blockaddress handling

[arsenm]

ping

[aeubanks]

llvm/test/tools/llvm-reduce/reduce-functions-blockaddress-wrong-function.ll

6–7 ↗

(On Diff #486279)

very weird, no idea why this would be happening. perhaps some cloning issue?

llvm/tools/llvm-reduce/deltas/ReduceFunctions.cpp

36

not sure I agree with this TODO

we should let the alias pass handle aliases and the instruction/globals pass handle blockaddresses, and keep this one simple

[arsenm]

ae6a5c1d08d8ac251a2b3dc08b023126b21b1b53

llvm/test/tools/llvm-reduce/reduce-functions-blockaddress.ll

+; RUN: llvm-reduce --delta-passes=functions --test FileCheck --test-arg --check-prefixes=INTERESTING --test-arg %s --test-arg --input-file %s -o %t
+; RUN: FileCheck --check-prefixes=RESULT --input-file=%t %s
+
+; Make sure we don't crash on blockaddress
+; TODO: Should be able to replace the blockaddresses with null too
+
+; INTERESTING: @blockaddr.table
+; INTERESTING: @blockaddr.table.addrspacecast
+
+; RESULT: @blockaddr.table = private unnamed_addr constant [2 x ptr] [ptr blockaddress(@foo, %L1), ptr blockaddress(@foo, %L2)]
+; RESULT: @blockaddr.table.addrspacecast = private unnamed_addr constant [2 x ptr addrspace(1)] [ptr addrspace(1) addrspacecast (ptr blockaddress(@foo_addrspacecast, %L1) to ptr addrspace(1)), ptr addrspace(1) addrspacecast (ptr blockaddress(@foo_addrspacecast, %L2) to ptr addrspace(1))]
+
[e-mail] = private unnamed_addr constant [2 x ptr] [ptr blockaddress(@foo, %L1), ptr blockaddress(@foo, %L2)]
+
[e-mail] = private unnamed_addr constant [2 x ptr addrspace(1)] [
+  ptr addrspace(1) addrspacecast (ptr blockaddress(@foo_addrspacecast, %L1) to ptr addrspace(1)),
+  ptr addrspace(1) addrspacecast (ptr blockaddress(@foo_addrspacecast, %L2) to ptr addrspace(1))
+]
+
+; RESULT: define i32 @foo(
+define i32 @foo(i64 %arg0) {
+entry:
+  %gep = getelementptr inbounds [2 x ptr], ptr @blockaddr.table, i64 0, i64 %arg0
+  %load = load ptr, ptr %gep, align 8
+  indirectbr ptr %load, [label %L2, label %L1]
+
+L1:
+  %phi = phi i32 [ 1, %L2 ], [ 2, %entry ]
+  ret i32 %phi
+
+L2:
+  br label %L1
+}
+
+; RESULT: define i32 @foo_addrspacecast(
+define i32 @foo_addrspacecast(i64 %arg0) {
+entry:
+  %gep = getelementptr inbounds [2 x ptr addrspace(1)], ptr @blockaddr.table.addrspacecast, i64 0, i64 %arg0
+  %load = load ptr addrspace(1), ptr %gep, align 8
+  indirectbr ptr addrspace(1) %load, [label %L2, label %L1]
+
+L1:
+  %phi = phi i32 [ 1, %L2 ], [ 2, %entry ]
+  ret i32 %phi
+
+L2:
+  br label %L1
+}

llvm/tools/llvm-reduce/deltas/ReduceFunctions.cpp

 /// that aren't inside any of the desired Chunks.
 static void extractFunctionsFromModule(Oracle &O, Module &Program) {
   // Record all out-of-chunk functions.
   SmallPtrSet<Constant *, 8> FuncsToRemove;
   for (Function &F : Program.functions()) {
     // 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.
-    if ((!F.isIntrinsic() || F.use_empty()) && !hasAliasUse(F) &&
+
+    // TODO: Could handle aliases and blockaddresses, we just need to replace
+    // those users.
+    if ((!F.isIntrinsic() || F.use_empty()) && !hasAliasOrBlockAddressUse(F) &&
         !O.shouldKeep())
       FuncsToRemove.insert(&F);
   }
 
   removeFromUsedLists(Program, [&FuncsToRemove](Constant *C) {
     return FuncsToRemove.count(C);
   });
 

llvm/tools/llvm-reduce/deltas/Utils.h

 #include "llvm/Support/CommandLine.h"
 
 namespace llvm {
 
 extern cl::opt<bool> Verbose;
 
 Value *getDefaultValue(Type *T);
 bool hasAliasUse(Function &F);
+bool hasAliasOrBlockAddressUse(Function &F);
 
 } // namespace llvm
 
 #endif

llvm/tools/llvm-reduce/deltas/Utils.cpp

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) || isa<GlobalIFunc>(U);
     });
 }
+
+bool llvm::hasAliasOrBlockAddressUse(Function &F) {
+  return any_of(F.users(), [](User *U) {
+    return isa<GlobalAlias, GlobalIFunc, BlockAddress>(U);
+  });
+}