Skip to content

Fix incorrect logic in maintaining the side-effect of compiler genera… #164

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed

Fix incorrect logic in maintaining the side-effect of compiler genera… #164

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
59 changes: 41 additions & 18 deletions llvm/lib/CodeGen/MachineOutliner.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@
//===----------------------------------------------------------------------===//
#include "llvm/CodeGen/MachineOutliner.h"
#include "llvm/ADT/DenseMap.h"
#include "llvm/ADT/SmallSet.h"
#include "llvm/ADT/Statistic.h"
#include "llvm/ADT/Twine.h"
#include "llvm/CodeGen/MachineFunction.h"
Expand Down Expand Up @@ -1245,31 +1246,53 @@ bool MachineOutliner::outline(Module &M,
// make sure that the ranges we yank things out of aren't wrong.
if (MBB.getParent()->getProperties().hasProperty(
MachineFunctionProperties::Property::TracksLiveness)) {
// Helper lambda for adding implicit def operands to the call
// The following code is to add implicit def operands to the call
// instruction. It also updates call site information for moved
// code.
auto CopyDefsAndUpdateCalls = [&CallInst](MachineInstr &MI) {
for (MachineOperand &MOP : MI.operands()) {
// Skip over anything that isn't a register.
if (!MOP.isReg())
continue;

// If it's a def, add it to the call instruction.
if (MOP.isDef())
CallInst->addOperand(MachineOperand::CreateReg(
MOP.getReg(), true, /* isDef = true */
true /* isImp = true */));
}
if (MI.shouldUpdateCallSiteInfo())
MI.getMF()->eraseCallSiteInfo(&MI);
};
SmallSet<Register, 2> UseRegs, DefRegs;
// Copy over the defs in the outlined range.
// First inst in outlined range <-- Anything that's defined in this
// ... .. range has to be added as an
// implicit Last inst in outlined range <-- def to the call
// instruction. Also remove call site information for outlined block
// of code.
std::for_each(CallInst, std::next(EndIt), CopyDefsAndUpdateCalls);
// of code. The exposed uses need to be copied in the outlined range.
for (MachineBasicBlock::reverse_iterator Iter = EndIt.getReverse(),
Last = std::next(CallInst.getReverse());
Iter != Last; Iter++) {
MachineInstr *MI = &*Iter;
for (MachineOperand &MOP : MI->operands()) {
// Skip over anything that isn't a register.
if (!MOP.isReg())
continue;

if (MOP.isDef()) {
// Introduce DefRegs set to skip the redundant register.
DefRegs.insert(MOP.getReg());
if (UseRegs.count(MOP.getReg()))
// Since the regiester is modeled as defined,
// it is not necessary to be put in use register set.
UseRegs.erase(MOP.getReg());
} else if (!MOP.isUndef()) {
// Any register which is not undefined should
// be put in the use register set.
UseRegs.insert(MOP.getReg());
}
}
if (MI->isCandidateForCallSiteEntry())
MI->getMF()->eraseCallSiteInfo(MI);
}

for (const Register &I : DefRegs)
// If it's a def, add it to the call instruction.
CallInst->addOperand(MachineOperand::CreateReg(
I, true, /* isDef = true */
true /* isImp = true */));

for (const Register &I : UseRegs)
// If it's a exposed use, add it to the call instruction.
CallInst->addOperand(
MachineOperand::CreateReg(I, false, /* isDef = false */
true /* isImp = true */));
}

// Erase from the point after where the call was inserted up to, and
Expand Down
8 changes: 4 additions & 4 deletions llvm/test/CodeGen/AArch64/machine-outliner-noreturn-save-lr.mir
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ body: |
; CHECK-LABEL: name: save_lr_1
; CHECK: liveins: $lr
; CHECK: $x0 = ORRXrs $xzr, $lr, 0
; 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
; 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
; CHECK: $lr = ORRXrs $xzr, $x0, 0
$w3 = ORRWri $wzr, 1
$w4 = ORRWri $wzr, 1
Expand All @@ -49,7 +49,7 @@ body: |
; CHECK-LABEL: name: save_lr_2
; CHECK: liveins: $lr
; CHECK: $x0 = ORRXrs $xzr, $lr, 0
; 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
; 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
; CHECK: $lr = ORRXrs $xzr, $x0, 0
$w3 = ORRWri $wzr, 1
$w4 = ORRWri $wzr, 1
Expand All @@ -71,7 +71,7 @@ body: |
; CHECK-LABEL: name: save_lr_3
; CHECK: liveins: $lr
; CHECK: $x0 = ORRXrs $xzr, $lr, 0
; 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
; 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
; CHECK: $lr = ORRXrs $xzr, $x0, 0
$w3 = ORRWri $wzr, 1
$w4 = ORRWri $wzr, 1
Expand All @@ -93,7 +93,7 @@ body: |
; CHECK-LABEL: name: save_lr_4
; CHECK: liveins: $lr
; CHECK: $x0 = ORRXrs $xzr, $lr, 0
; 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
; 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
; CHECK: $lr = ORRXrs $xzr, $x0, 0
$w3 = ORRWri $wzr, 1
$w4 = ORRWri $wzr, 1
Expand Down
32 changes: 32 additions & 0 deletions llvm/test/CodeGen/AArch64/machine-outliner-side-effect.mir
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
# RUN: llc -mtriple=aarch64 -run-pass=machine-outliner -verify-machineinstrs %s -o - | FileCheck %s

# The test checks whether the compiler updates the side effect of function @OUTLINED_FUNCTION_0 by adding the use of register x20.

--- |
declare void @spam() local_unnamed_addr
define void @baz() optsize minsize noredzone { ret void }
...
---
name: baz
tracksRegLiveness: true
body: |
bb.0:
liveins: $x0, $x20

$x0 = COPY renamable $x20
BL @spam, csr_aarch64_aapcs, implicit-def dead $lr, implicit $sp, implicit $x0, implicit-def $sp, implicit-def $x0
renamable $x21 = COPY $x0

$x0 = COPY renamable $x20
BL @spam, csr_aarch64_aapcs, implicit-def dead $lr, implicit $sp, implicit $x0, implicit-def $sp, implicit-def $x0
renamable $x22 = COPY $x0

$x0 = COPY killed renamable $x20
BL @spam, csr_aarch64_aapcs, implicit-def dead $lr, implicit $sp, implicit $x0, implicit-def $sp, implicit-def $x0
renamable $x3 = COPY $x0

RET_ReallyLR

...

# CHECK: BL @OUTLINED_FUNCTION_0, {{.*}}, implicit $x20, {{.*}}