Skip to content

Commit fc6fda9

Browse files
jinlin-bayareajinlin-bayarea
committed
Fix incorrect logic in maintaining the side-effect of compiler generated outliner functions
Summary: Fix incorrect logic in maintaining the side-effect of compiler generated outliner functions by adding the up-exposed uses. Reviewers: paquette, tellenbach Reviewed By: paquette Subscribers: aemerson, lebedev.ri, hiraditya, llvm-commits, jinlin Tags: #llvm Differential Revision: https://reviews.llvm.org/D71217
1 parent 5dadf57 commit fc6fda9

File tree

3 files changed

+77
-22
lines changed

3 files changed

+77
-22
lines changed

‎llvm/lib/CodeGen/MachineOutliner.cpp‎

Lines changed: 41 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,7 @@
5656
//===----------------------------------------------------------------------===//
5757
#include "llvm/CodeGen/MachineOutliner.h"
5858
#include "llvm/ADT/DenseMap.h"
59+
#include "llvm/ADT/SmallSet.h"
5960
#include "llvm/ADT/Statistic.h"
6061
#include "llvm/ADT/Twine.h"
6162
#include "llvm/CodeGen/MachineFunction.h"
@@ -1245,31 +1246,53 @@ bool MachineOutliner::outline(Module &M,
12451246
// make sure that the ranges we yank things out of aren't wrong.
12461247
if (MBB.getParent()->getProperties().hasProperty(
12471248
MachineFunctionProperties::Property::TracksLiveness)) {
1248-
// Helper lambda for adding implicit def operands to the call
1249+
// The following code is to add implicit def operands to the call
12491250
// instruction. It also updates call site information for moved
12501251
// code.
1251-
auto CopyDefsAndUpdateCalls = [&CallInst](MachineInstr &MI) {
1252-
for (MachineOperand &MOP : MI.operands()) {
1253-
// Skip over anything that isn't a register.
1254-
if (!MOP.isReg())
1255-
continue;
1256-
1257-
// If it's a def, add it to the call instruction.
1258-
if (MOP.isDef())
1259-
CallInst->addOperand(MachineOperand::CreateReg(
1260-
MOP.getReg(), true, /* isDef = true */
1261-
true /* isImp = true */));
1262-
}
1263-
if (MI.shouldUpdateCallSiteInfo())
1264-
MI.getMF()->eraseCallSiteInfo(&MI);
1265-
};
1252+
SmallSet<Register, 2> UseRegs, DefRegs;
12661253
// Copy over the defs in the outlined range.
12671254
// First inst in outlined range <-- Anything that's defined in this
12681255
// ... .. range has to be added as an
12691256
// implicit Last inst in outlined range <-- def to the call
12701257
// instruction. Also remove call site information for outlined block
1271-
// of code.
1272-
std::for_each(CallInst, std::next(EndIt), CopyDefsAndUpdateCalls);
1258+
// of code. The exposed uses need to be copied in the outlined range.
1259+
for (MachineBasicBlock::reverse_iterator Iter = EndIt.getReverse(),
1260+
Last = std::next(CallInst.getReverse());
1261+
Iter != Last; Iter++) {
1262+
MachineInstr *MI = &*Iter;
1263+
for (MachineOperand &MOP : MI->operands()) {
1264+
// Skip over anything that isn't a register.
1265+
if (!MOP.isReg())
1266+
continue;
1267+
1268+
if (MOP.isDef()) {
1269+
// Introduce DefRegs set to skip the redundant register.
1270+
DefRegs.insert(MOP.getReg());
1271+
if (UseRegs.count(MOP.getReg()))
1272+
// Since the regiester is modeled as defined,
1273+
// it is not necessary to be put in use register set.
1274+
UseRegs.erase(MOP.getReg());
1275+
} else if (!MOP.isUndef()) {
1276+
// Any register which is not undefined should
1277+
// be put in the use register set.
1278+
UseRegs.insert(MOP.getReg());
1279+
}
1280+
}
1281+
if (MI->isCandidateForCallSiteEntry())
1282+
MI->getMF()->eraseCallSiteInfo(MI);
1283+
}
1284+
1285+
for (const Register &I : DefRegs)
1286+
// If it's a def, add it to the call instruction.
1287+
CallInst->addOperand(MachineOperand::CreateReg(
1288+
I, true, /* isDef = true */
1289+
true /* isImp = true */));
1290+
1291+
for (const Register &I : UseRegs)
1292+
// If it's a exposed use, add it to the call instruction.
1293+
CallInst->addOperand(
1294+
MachineOperand::CreateReg(I, false, /* isDef = false */
1295+
true /* isImp = true */));
12731296
}
12741297

12751298
// Erase from the point after where the call was inserted up to, and

‎llvm/test/CodeGen/AArch64/machine-outliner-noreturn-save-lr.mir‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ body: |
2727
; CHECK-LABEL: name: save_lr_1
2828
; CHECK: liveins: $lr
2929
; CHECK: $x0 = ORRXrs $xzr, $lr, 0
30-
; CHECK: BL @OUTLINED_FUNCTION_0, implicit-def $lr, implicit $sp, implicit-def $lr, implicit-def $lr, implicit-def $w3, implicit-def $w4, implicit-def $w5, implicit-def $w6
30+
; CHECK: BL @OUTLINED_FUNCTION_0, implicit-def $lr, implicit $sp, implicit-def $lr, implicit-def $w3, implicit-def $w4, implicit-def $w5, implicit-def $w6, implicit $sp, implicit $wzr, implicit $xzr, implicit $x0
3131
; CHECK: $lr = ORRXrs $xzr, $x0, 0
3232
$w3 = ORRWri $wzr, 1
3333
$w4 = ORRWri $wzr, 1
@@ -49,7 +49,7 @@ body: |
4949
; CHECK-LABEL: name: save_lr_2
5050
; CHECK: liveins: $lr
5151
; CHECK: $x0 = ORRXrs $xzr, $lr, 0
52-
; CHECK: BL @OUTLINED_FUNCTION_0, implicit-def $lr, implicit $sp, implicit-def $lr, implicit-def $lr, implicit-def $w3, implicit-def $w4, implicit-def $w5, implicit-def $w6
52+
; CHECK: BL @OUTLINED_FUNCTION_0, implicit-def $lr, implicit $sp, implicit-def $lr, implicit-def $w3, implicit-def $w4, implicit-def $w5, implicit-def $w6, implicit $sp, implicit $wzr, implicit $xzr, implicit $x0
5353
; CHECK: $lr = ORRXrs $xzr, $x0, 0
5454
$w3 = ORRWri $wzr, 1
5555
$w4 = ORRWri $wzr, 1
@@ -71,7 +71,7 @@ body: |
7171
; CHECK-LABEL: name: save_lr_3
7272
; CHECK: liveins: $lr
7373
; CHECK: $x0 = ORRXrs $xzr, $lr, 0
74-
; CHECK: BL @OUTLINED_FUNCTION_0, implicit-def $lr, implicit $sp, implicit-def $lr, implicit-def $lr, implicit-def $w3, implicit-def $w4, implicit-def $w5, implicit-def $w6
74+
; CHECK: BL @OUTLINED_FUNCTION_0, implicit-def $lr, implicit $sp, implicit-def $lr, implicit-def $w3, implicit-def $w4, implicit-def $w5, implicit-def $w6, implicit $sp, implicit $wzr, implicit $xzr, implicit $x0
7575
; CHECK: $lr = ORRXrs $xzr, $x0, 0
7676
$w3 = ORRWri $wzr, 1
7777
$w4 = ORRWri $wzr, 1
@@ -93,7 +93,7 @@ body: |
9393
; CHECK-LABEL: name: save_lr_4
9494
; CHECK: liveins: $lr
9595
; CHECK: $x0 = ORRXrs $xzr, $lr, 0
96-
; CHECK: BL @OUTLINED_FUNCTION_0, implicit-def $lr, implicit $sp, implicit-def $lr, implicit-def $lr, implicit-def $w3, implicit-def $w4, implicit-def $w5, implicit-def $w6
96+
; CHECK: BL @OUTLINED_FUNCTION_0, implicit-def $lr, implicit $sp, implicit-def $lr, implicit-def $w3, implicit-def $w4, implicit-def $w5, implicit-def $w6, implicit $sp, implicit $wzr, implicit $xzr, implicit $x0
9797
; CHECK: $lr = ORRXrs $xzr, $x0, 0
9898
$w3 = ORRWri $wzr, 1
9999
$w4 = ORRWri $wzr, 1

‎llvm/test/CodeGen/AArch64/machine-outliner-side-effect.mir‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
# RUN: llc -mtriple=aarch64 -run-pass=machine-outliner -verify-machineinstrs %s -o - | FileCheck %s
2+
3+
# The test checks whether the compiler updates the side effect of function @OUTLINED_FUNCTION_0 by adding the use of register x20.
4+
5+
--- |
6+
declare void @spam() local_unnamed_addr
7+
define void @baz() optsize minsize noredzone { ret void }
8+
...
9+
---
10+
name: baz
11+
tracksRegLiveness: true
12+
body: |
13+
bb.0:
14+
liveins: $x0, $x20
15+
16+
$x0 = COPY renamable $x20
17+
BL @spam, csr_aarch64_aapcs, implicit-def dead $lr, implicit $sp, implicit $x0, implicit-def $sp, implicit-def $x0
18+
renamable $x21 = COPY $x0
19+
20+
$x0 = COPY renamable $x20
21+
BL @spam, csr_aarch64_aapcs, implicit-def dead $lr, implicit $sp, implicit $x0, implicit-def $sp, implicit-def $x0
22+
renamable $x22 = COPY $x0
23+
24+
$x0 = COPY killed renamable $x20
25+
BL @spam, csr_aarch64_aapcs, implicit-def dead $lr, implicit $sp, implicit $x0, implicit-def $sp, implicit-def $x0
26+
renamable $x3 = COPY $x0
27+
28+
RET_ReallyLR
29+
30+
...
31+
32+
# CHECK: BL @OUTLINED_FUNCTION_0, {{.*}}, implicit $x20, {{.*}}

0 commit comments

Comments
 (0)