MergeFunc: Don't merge functions with different range metadata on call/invoke

[dotdash]

Summary

Since r211281 call and invoke can have range metadata which must be
taken into consideration when comparing such instructions.

[dyatkovskiy]

Hi Björn,

The changes itself looks fine, but I have to check tests as well.. I'm moving on this week, so I can answer you in Wednesday only.
Thanks for working on it.
By the way, we finished our O(n*log(n)) patch series. You could look into FunctionPtr. Perhaps it had to be renamed to FunctionInfo or something like this.
Anyways in this structure you could keep additional data for instructions you need (such as ranges that were merged currently). I try to think, how to bring it into most generic form. Perhaps that's not only about ranges and tbaa..

Thanks!

[dyatkovskiy]

Changes in MergeFunctions.cpp - LGTM
call-with-ranges.ll - LGTM
invoke-with-ranges.ll - read inline comment, otherwise - fine.

test/Transforms/MergeFunc/invoke-with-ranges.ll

11

Can we avoid bitcasts?

[dotdash]

Updated the tests for invoke-with-ranges to avoid the bitcasts

[dyatkovskiy]

LGTM,
now let's wait Nick and talk to him :-)

[dyatkovskiy]

Hi Nick,
can I commit patch for Björn?
Thanks!
-Stepan

Stepan Dyatkovskiy wrote:

LGTM

http://reviews.llvm.org/D4246

[dyatkovskiy]

ping
Stepan Dyatkovskiy wrote:

Hi Nick,
can I commit patch for Björn?
Thanks!
-Stepan

Stepan Dyatkovskiy wrote:

LGTM

http://reviews.llvm.org/D4246

[nicholas]

Optional: I think the two tests could be merged into one file. Either way, LGTM.

[dyatkovskiy]

Committed as r213060
Thanks for working on it!

lib/Transforms/IPO/MergeFunctions.cpp

if (const StoreInst *SI = dyn_cast<StoreInst>(L)) {
     return cmpNumbers(SI->getSynchScope(), cast<StoreInst>(R)->getSynchScope());
   }
   if (const CmpInst *CI = dyn_cast<CmpInst>(L))
     return cmpNumbers(CI->getPredicate(), cast<CmpInst>(R)->getPredicate());
   if (const CallInst *CI = dyn_cast<CallInst>(L)) {
     if (int Res = cmpNumbers(CI->getCallingConv(),
                              cast<CallInst>(R)->getCallingConv()))
       return Res;
-    return cmpAttrs(CI->getAttributes(), cast<CallInst>(R)->getAttributes());
+    if (int Res = cmpAttrs(CI->getAttributes(), cast<CallInst>(R)->getAttributes()))
+      return Res;
+    return cmpNumbers((uint64_t)CI->getMetadata(LLVMContext::MD_range),
+                      (uint64_t)cast<CallInst>(R)->getMetadata(LLVMContext::MD_range));
   }
   if (const InvokeInst *CI = dyn_cast<InvokeInst>(L)) {
     if (int Res = cmpNumbers(CI->getCallingConv(),
                              cast<InvokeInst>(R)->getCallingConv()))
       return Res;
-    return cmpAttrs(CI->getAttributes(), cast<InvokeInst>(R)->getAttributes());
+    if (int Res = cmpAttrs(CI->getAttributes(), cast<InvokeInst>(R)->getAttributes()))
+      return Res;
+    return cmpNumbers((uint64_t)CI->getMetadata(LLVMContext::MD_range),
+                      (uint64_t)cast<InvokeInst>(R)->getMetadata(LLVMContext::MD_range));
   }
   if (const InsertValueInst *IVI = dyn_cast<InsertValueInst>(L)) {
     ArrayRef<unsigned> LIndices = IVI->getIndices();
     ArrayRef<unsigned> RIndices = cast<InsertValueInst>(R)->getIndices();
     if (int Res = cmpNumbers(LIndices.size(), RIndices.size()))
       return Res;
     for (size_t i = 0, e = LIndices.size(); i != e; ++i) {
       if (int Res = cmpNumbers(LIndices[i], RIndices[i]))

test/Transforms/MergeFunc/call-with-ranges.ll

+; RUN: opt -mergefunc -S < %s | FileCheck %s
+
+define i8 @call_with_range() {
+  bitcast i8 0 to i8 ; dummy to make the function large enough
+  %out = call i8 @dummy(), !range !0
+  ret i8 %out
+}
+
+define i8 @call_no_range() {
+; CHECK-LABEL: @call_no_range
+; CHECK-NEXT: bitcast i8 0 to i8
+; CHECK-NEXT: %out = call i8 @dummy()
+; CHECK-NEXT: ret i8 %out
+  bitcast i8 0 to i8
+  %out = call i8 @dummy()
+  ret i8 %out
+}
+
+define i8 @call_different_range() {
+; CHECK-LABEL: @call_different_range
+; CHECK-NEXT: bitcast i8 0 to i8
+; CHECK-NEXT: %out = call i8 @dummy(), !range !1
+; CHECK-NEXT: ret i8 %out
+  bitcast i8 0 to i8
+  %out = call i8 @dummy(), !range !1
+  ret i8 %out
+}
+
+define i8 @call_same_range() {
+; CHECK-LABEL: @call_same_range
+; CHECK: tail call i8 @call_with_range
+  bitcast i8 0 to i8
+  %out = call i8 @dummy(), !range !0
+  ret i8 %out
+}
+
+declare i8 @dummy();
+declare i32 @__gxx_personality_v0(...)
+
+!0 = metadata !{i8 0, i8 2}
+!1 = metadata !{i8 5, i8 7}

test/Transforms/MergeFunc/invoke-with-ranges.ll

+; RUN: opt -mergefunc -S < %s | FileCheck %s
+
+define i8 @invoke_with_range() {
+  %out = invoke i8 @dummy() to label %next unwind label %lpad, !range !0
+
+next:
+  ret i8 %out
+
+lpad:
+  %pad = landingpad { i8*, i32 } personality i8* undef cleanup
+  resume { i8*, i32 } zeroinitializer
+}
+
+define i8 @invoke_no_range() {
+; CHECK-LABEL: @invoke_no_range()
+; CHECK-NEXT: invoke i8 @dummy
+  %out = invoke i8 @dummy() to label %next unwind label %lpad
+
+next:
+  ret i8 %out
+
+lpad:
+  %pad = landingpad { i8*, i32 } personality i8* undef cleanup
+  resume { i8*, i32 } zeroinitializer
+}
+
+define i8 @invoke_different_range() {
+; CHECK-LABEL: @invoke_different_range()
+; CHECK-NEXT: invoke i8 @dummy
+  %out = invoke i8 @dummy() to label %next unwind label %lpad, !range !1
+
+next:
+  ret i8 %out
+
+lpad:
+  %pad = landingpad { i8*, i32 } personality i8* undef cleanup
+  resume { i8*, i32 } zeroinitializer
+}
+
+define i8 @invoke_same_range() {
+; CHECK-LABEL: @invoke_same_range()
+; CHECK: tail call i8 @invoke_with_range()
+  %out = invoke i8 @dummy() to label %next unwind label %lpad, !range !0
+
+next:
+  ret i8 %out
+
+lpad:
+  %pad = landingpad { i8*, i32 } personality i8* undef cleanup
+  resume { i8*, i32 } zeroinitializer
+}
+
+declare i8 @dummy();
+
+!0 = metadata !{i8 0, i8 2}
+!1 = metadata !{i8 5, i8 7}