[IRSim][IROutliner] Allowing call instructions to be outlined.

[AndrewLitteken]

Summary

We add an extra check to make sure that we do not outline calls to indirect functions, but outline whatever the IRSimilarityIdentifier finds with respect to calls.

Tests:

• Removing test/Transforms/IROutliner/illegal-calls.ll
• Adding test/Transforms/IROutliner/outlining-calls.ll
• Adding test/Transforms/IROutliner/illegal-indirect-calls.ll

[jroelofs]

llvm/include/llvm/Transforms/IPO/IROutliner.h

324–331

I don't follow the logic behind needing a name for this to work. ISTM you could pass the callee pointer as another new arg, and everything else would Just Work (TM). What am I missing?

[AndrewLitteken]

llvm/include/llvm/Transforms/IPO/IROutliner.h

324–331

I don’t think you’re missing anything on that front. I just hadn’t considered writing it like that before. The same name just ensures that we are outlining the exact same function in both cases without any sort of argument handling, if it’s indirect, we can’t guarantee that.

However I think that it might be best to handle indirect calls in a separate patch later on since it adds a layer of complexity onto handling the arguments (see https://reviews.llvm.org/D87294 for an example of how it would be handled).

[jroelofs]

LGTM

llvm/include/llvm/Transforms/IPO/IROutliner.h

324–331

👍

llvm/include/llvm/Transforms/IPO/IROutliner.h

struct InstructionAllowed : public InstVisitor<InstructionAllowed, bool> {
     bool visitFuncletPadInst(FuncletPadInst &FPI) { return false; }
     // DebugInfo should be included in the regions, but should not be
     // analyzed for similarity as it has no bearing on the outcome of the
     // program.
     bool visitDbgInfoIntrinsic(DbgInfoIntrinsic &DII) { return true; }
     // TODO: Handle specific intrinsics individually from those that can be
     // handled.
     bool IntrinsicInst(IntrinsicInst &II) { return false; }
-    // TODO: Handle CallInsts, there will need to be handling for special kinds
-    // of calls, as well as calls to intrinsics.
-    bool visitCallInst(CallInst &CI) { return false; }
+    // We only handle CallInsts that are not indirect, since we cannot guarantee
+    // that they have a name in these cases.
+    bool visitCallInst(CallInst &CI) {
+      Function *F = CI.getCalledFunction();
+      if (!F || CI.isIndirectCall() || !F->hasName())
+        return false;
+      return true;
+    }
     // TODO: Handle FreezeInsts.  Since a frozen value could be frozen inside
     // the outlined region, and then returned as an output, this will have to be
     // handled differently.
     bool visitFreezeInst(FreezeInst &CI) { return false; }
     // TODO: We do not current handle similarity that changes the control flow.
     bool visitInvokeInst(InvokeInst &II) { return false; }
     // TODO: We do not current handle similarity that changes the control flow.
     bool visitCallBrInst(CallBrInst &CBI) { return false; }

llvm/test/Transforms/IROutliner/illegal-indirect-calls.ll

 ; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
 ; RUN: opt -S -verify -iroutliner -ir-outlining-no-cost < %s | FileCheck %s
 
-; This test checks that we do not outline calls.  Special calls, such as
-; indirect or nameless calls require extra handling to ensure that there
-; are no inconsistencies when outlining and consolidating regions.
+; This test checks that we do not outline indirect calls.  We cannot guarantee
+; that we have the same name in these cases, so two indirect calls cannot
+; be considered similar.
 
 declare void @f1(i32*, i32*);
+declare void @f2(i32*, i32*);
 
-define void @outline_constants1() {
-; CHECK-LABEL: @outline_constants1(
+define void @function1(void()* %func) {
+; CHECK-LABEL: @function1(
 ; CHECK-NEXT:  entry:
 ; CHECK-NEXT:    [[A:%.*]] = alloca i32, align 4
 ; CHECK-NEXT:    [[B:%.*]] = alloca i32, align 4
 ; CHECK-NEXT:    [[C:%.*]] = alloca i32, align 4
-; CHECK-NEXT:    store i32 2, i32* [[A]], align 4
-; CHECK-NEXT:    store i32 3, i32* [[B]], align 4
-; CHECK-NEXT:    store i32 4, i32* [[C]], align 4
-; CHECK-NEXT:    call void @f1(i32* [[A]], i32* [[B]])
+; CHECK-NEXT:    call void @outlined_ir_func_1(i32* [[A]], i32* [[B]], i32* [[C]])
+; CHECK-NEXT:    call void [[FUNC:%.*]]()
 ; CHECK-NEXT:    call void @outlined_ir_func_0(i32* [[A]], i32* [[B]], i32* [[C]])
 ; CHECK-NEXT:    ret void
 ;
 entry:
   %a = alloca i32, align 4
   %b = alloca i32, align 4
   %c = alloca i32, align 4
   store i32 2, i32* %a, align 4
   store i32 3, i32* %b, align 4
   store i32 4, i32* %c, align 4
-  call void @f1(i32* %a, i32* %b)
+  call void %func()
   %al = load i32, i32* %a
   %bl = load i32, i32* %b
   %cl = load i32, i32* %c
   ret void
 }
 
-define void @outline_constants2() {
-; CHECK-LABEL: @outline_constants2(
+define void @function2(void()* %func) {
+; CHECK-LABEL: @function2(
 ; CHECK-NEXT:  entry:
 ; CHECK-NEXT:    [[A:%.*]] = alloca i32, align 4
 ; CHECK-NEXT:    [[B:%.*]] = alloca i32, align 4
 ; CHECK-NEXT:    [[C:%.*]] = alloca i32, align 4
-; CHECK-NEXT:    store i32 2, i32* [[A]], align 4
-; CHECK-NEXT:    store i32 3, i32* [[B]], align 4
-; CHECK-NEXT:    store i32 4, i32* [[C]], align 4
-; CHECK-NEXT:    call void @f1(i32* [[A]], i32* [[B]])
+; CHECK-NEXT:    call void @outlined_ir_func_1(i32* [[A]], i32* [[B]], i32* [[C]])
+; CHECK-NEXT:    call void [[FUNC:%.*]]()
 ; CHECK-NEXT:    call void @outlined_ir_func_0(i32* [[A]], i32* [[B]], i32* [[C]])
 ; CHECK-NEXT:    ret void
 ;
 entry:
   %a = alloca i32, align 4
   %b = alloca i32, align 4
   %c = alloca i32, align 4
   store i32 2, i32* %a, align 4
   store i32 3, i32* %b, align 4
   store i32 4, i32* %c, align 4
-  call void @f1(i32* %a, i32* %b)
+  call void %func()
   %al = load i32, i32* %a
   %bl = load i32, i32* %b
   %cl = load i32, i32* %c
   ret void
 }

llvm/test/Transforms/IROutliner/outlining-calls.ll

+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
+; RUN: opt -S -verify -iroutliner < %s | FileCheck %s
+
+; This test checks that we do can outline calls, but only if they have the same
+; function type and the same name.
+
+declare void @f1(i32*, i32*);
+declare void @f2(i32*, i32*);
+
+define void @function1() {
+; CHECK-LABEL: @function1(
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:    [[A:%.*]] = alloca i32, align 4
+; CHECK-NEXT:    [[B:%.*]] = alloca i32, align 4
+; CHECK-NEXT:    [[C:%.*]] = alloca i32, align 4
+; CHECK-NEXT:    call void @outlined_ir_func_0(i32* [[A]], i32* [[B]], i32* [[C]])
+; CHECK-NEXT:    ret void
+;
+entry:
+  %a = alloca i32, align 4
+  %b = alloca i32, align 4
+  %c = alloca i32, align 4
+  store i32 2, i32* %a, align 4
+  store i32 3, i32* %b, align 4
+  store i32 4, i32* %c, align 4
+  call void @f1(i32* %a, i32* %b)
+  %al = load i32, i32* %a
+  %bl = load i32, i32* %b
+  %cl = load i32, i32* %c
+  ret void
+}
+
+define void @function2() {
+; CHECK-LABEL: @function2(
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:    [[A:%.*]] = alloca i32, align 4
+; CHECK-NEXT:    [[B:%.*]] = alloca i32, align 4
+; CHECK-NEXT:    [[C:%.*]] = alloca i32, align 4
+; CHECK-NEXT:    call void @outlined_ir_func_0(i32* [[A]], i32* [[B]], i32* [[C]])
+; CHECK-NEXT:    ret void
+;
+entry:
+  %a = alloca i32, align 4
+  %b = alloca i32, align 4
+  %c = alloca i32, align 4
+  store i32 2, i32* %a, align 4
+  store i32 3, i32* %b, align 4
+  store i32 4, i32* %c, align 4
+  call void @f1(i32* %a, i32* %b)
+  %al = load i32, i32* %a
+  %bl = load i32, i32* %b
+  %cl = load i32, i32* %c
+  ret void
+}
+
+define void @function3() {
+; CHECK-LABEL: @function3(
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:    [[A:%.*]] = alloca i32, align 4
+; CHECK-NEXT:    [[B:%.*]] = alloca i32, align 4
+; CHECK-NEXT:    [[C:%.*]] = alloca i32, align 4
+; CHECK-NEXT:    store i32 2, i32* [[A]], align 4
+; CHECK-NEXT:    store i32 3, i32* [[B]], align 4
+; CHECK-NEXT:    store i32 4, i32* [[C]], align 4
+; CHECK-NEXT:    call void @f2(i32* [[A]], i32* [[B]])
+; CHECK-NEXT:    [[AL:%.*]] = load i32, i32* [[A]], align 4
+; CHECK-NEXT:    [[BL:%.*]] = load i32, i32* [[B]], align 4
+; CHECK-NEXT:    [[CL:%.*]] = load i32, i32* [[C]], align 4
+; CHECK-NEXT:    ret void
+;
+entry:
+  %a = alloca i32, align 4
+  %b = alloca i32, align 4
+  %c = alloca i32, align 4
+  store i32 2, i32* %a, align 4
+  store i32 3, i32* %b, align 4
+  store i32 4, i32* %c, align 4
+  call void @f2(i32* %a, i32* %b)
+  %al = load i32, i32* %a
+  %bl = load i32, i32* %b
+  %cl = load i32, i32* %c
+  ret void
+}
+
+; CHECK: define internal void @outlined_ir_func_0(i32* [[ARG0:%.*]], i32* [[ARG1:%.*]], i32* [[ARG2:%.*]])
+; CHECK: entry_to_outline:
+; CHECK-NEXT:    store i32 2, i32* [[ARG0]], align 4
+; CHECK-NEXT:    store i32 3, i32* [[ARG1]], align 4
+; CHECK-NEXT:    store i32 4, i32* [[ARG2]], align 4
+; CHECK-NEXT:    call void @f1(i32* [[ARG0]], i32* [[ARG1]])
+; CHECK-NEXT:    [[AL:%.*]] = load i32, i32* [[ARG0]], align 4
+; CHECK-NEXT:    [[BL:%.*]] = load i32, i32* [[ARG1]], align 4
+; CHECK-NEXT:    [[CL:%.*]] = load i32, i32* [[ARG2]], align 4