Fix assertion in FunctionComparator::cmpInlineAsm

[nikic]

Summary

Fixes bug https://bugs.llvm.org/show_bug.cgi?id=37339.

InlineAsm is only uniqued if the FunctionTypes are exactly the
same, while cmpTypes() for example considers all pointer types
in the default address space to be the same. For this reason
the end of cmpInlineAsm() can be reached.

This patch replaces the unreachable assertion with a check that
the function types are not identical.

[jfb]

test/Transforms/MergeFunc/inline-asm.ll

5 ↗

(On Diff #145377)

Can you add a separate test where the function types are the same, but the targets are different (i.e. not null)?

[nikic]

Updated test to test both different pointer types as inline asm argument types and return types.

I've also adjusted the test to make the functions internal and have a separate function call them. This avoids the need for the somewhat non-obvious call void @stuff calls to satisfy minimum function size.

[nikic]

Sorry, I misunderstood your previous comment and thought you were referring to the return type. I've added two variants to the test now, one where a non-null argument is used and it can be merged and one where it can't be merged.

[nox]

Can anyone land that?

llvm/trunk/lib/Transforms/Utils/FunctionComparator.cpp

int FunctionComparator::cmpInlineAsm(const InlineAsm *L,
   if (int Res = cmpMem(L->getConstraintString(), R->getConstraintString()))
     return Res;
   if (int Res = cmpNumbers(L->hasSideEffects(), R->hasSideEffects()))
     return Res;
   if (int Res = cmpNumbers(L->isAlignStack(), R->isAlignStack()))
     return Res;
   if (int Res = cmpNumbers(L->getDialect(), R->getDialect()))
     return Res;
-  llvm_unreachable("InlineAsm blocks were not uniqued.");
+  assert(L->getFunctionType() != R->getFunctionType());
   return 0;
 }
 
 /// Compare two values used by the two functions under pair-wise comparison. If
 /// this is the first time the values are seen, they're added to the mapping so
 /// that we will detect mismatches on next use.
 /// See comments in declaration for more details.
 int FunctionComparator::cmpValues(const Value *L, const Value *R) const {

llvm/trunk/test/Transforms/MergeFunc/inline-asm.ll

+; RUN: opt -mergefunc -S < %s | FileCheck %s
+
+; CHECK-LABEL: @int_ptr_arg_different
+; CHECK-NEXT: call void asm
+
+; CHECK-LABEL: @int_ptr_arg_same
+; CHECK-NEXT: %2 = bitcast i32* %0 to float*
+; CHECK-NEXT: tail call void @float_ptr_arg_same(float* %2)
+
+; CHECK-LABEL: @int_ptr_null
+; CHECK-NEXT: tail call void @float_ptr_null()
+
+; Used to satisfy minimum size limit
+declare void @stuff()
+
+; Can be merged
+define void @float_ptr_null() {
+  call void asm "nop", "r"(float* null)
+  call void @stuff()
+  ret void
+}
+
+define void @int_ptr_null() {
+  call void asm "nop", "r"(i32* null)
+  call void @stuff()
+  ret void
+}
+
+; Can be merged (uses same argument differing by pointer type)
+define void @float_ptr_arg_same(float*) {
+  call void asm "nop", "r"(float* %0)
+  call void @stuff()
+  ret void
+}
+
+define void @int_ptr_arg_same(i32*) {
+  call void asm "nop", "r"(i32* %0)
+  call void @stuff()
+  ret void
+}
+
+; Can not be merged (uses different arguments)
+define void @float_ptr_arg_different(float*, float*) {
+  call void asm "nop", "r"(float* %0)
+  call void @stuff()
+  ret void
+}
+
+define void @int_ptr_arg_different(i32*, i32*) {
+  call void asm "nop", "r"(i32* %1)
+  call void @stuff()
+  ret void
+}