llvm-reduce: Fix assertion on blockaddress during function reduction
authorMatt Arsenault <Matthew.Arsenault@amd.com>
Tue, 3 Jan 2023 17:45:42 +0000 (12:45 -0500)
committerMatt Arsenault <arsenm2@gmail.com>
Wed, 11 Jan 2023 13:10:04 +0000 (08:10 -0500)
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.

https://reviews.llvm.org/D140909

llvm/test/tools/llvm-reduce/reduce-functions-blockaddress-wrong-function.ll [new file with mode: 0644]
llvm/test/tools/llvm-reduce/reduce-functions-blockaddress.ll [new file with mode: 0644]
llvm/tools/llvm-reduce/deltas/ReduceFunctions.cpp
llvm/tools/llvm-reduce/deltas/Utils.cpp
llvm/tools/llvm-reduce/deltas/Utils.h

diff --git a/llvm/test/tools/llvm-reduce/reduce-functions-blockaddress-wrong-function.ll b/llvm/test/tools/llvm-reduce/reduce-functions-blockaddress-wrong-function.ll
new file mode 100644 (file)
index 0000000..70aeb1f
--- /dev/null
@@ -0,0 +1,38 @@
+; 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
+
+; FIXME: This testcase exhibits nonsensical behavior. The first
+; function has blockaddress references. When the second function is
+; deleted, it causes the blockreferences from the first to be replaced
+; with inttoptr.
+
+; INTERESTING: @blockaddr.table.other
+
+; RESULT: @blockaddr.table.other = private unnamed_addr constant [2 x ptr] [ptr inttoptr (i32 1 to ptr), ptr inttoptr (i32 1 to ptr)]
+
+@blockaddr.table.other = private unnamed_addr constant [2 x ptr] [ptr blockaddress(@bar, %L1), ptr blockaddress(@bar, %L2)]
+
+
+; RESULT: define i32 @bar(
+define i32 @bar(i64 %arg0) {
+entry:
+  %gep = getelementptr inbounds [2 x ptr], ptr @blockaddr.table.other, 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-NOT: @unused
+define void @unused() {
+entry:
+  br label %exit
+
+exit:
+  ret void
+}
diff --git a/llvm/test/tools/llvm-reduce/reduce-functions-blockaddress.ll b/llvm/test/tools/llvm-reduce/reduce-functions-blockaddress.ll
new file mode 100644 (file)
index 0000000..4f84e59
--- /dev/null
@@ -0,0 +1,48 @@
+; 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))]
+
+@blockaddr.table = private unnamed_addr constant [2 x ptr] [ptr blockaddress(@foo, %L1), ptr blockaddress(@foo, %L2)]
+
+@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))
+]
+
+; 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
+}
index ef208ad..0c1ec50 100644 (file)
@@ -32,7 +32,7 @@ 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.
-    if ((!F.isIntrinsic() || F.use_empty()) && !hasAliasUse(F) &&
+    if ((!F.isIntrinsic() || F.use_empty()) && !hasAliasOrBlockAddressUse(F) &&
         !O.shouldKeep())
       FuncsToRemove.insert(&F);
   }
index a7c9528..669b9db 100644 (file)
@@ -32,3 +32,9 @@ bool llvm::hasAliasUse(Function &F) {
       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);
+  });
+}
index 91041e2..e94aee5 100644 (file)
@@ -23,6 +23,7 @@ extern cl::opt<bool> Verbose;
 
 Value *getDefaultValue(Type *T);
 bool hasAliasUse(Function &F);
+bool hasAliasOrBlockAddressUse(Function &F);
 
 } // namespace llvm