This is an archive of the discontinued LLVM Phabricator instance.

Differential D98811

[GlobalISel] Don't DCE LIFETIME_START/LIFETIME_END markers.
ClosedPublic

Authored by aemerson on Mar 17 2021, 1:29 PM.

Details

Reviewers
paquette
arsenm
Commits
rG28963d895b52: [GlobalISel] Don't DCE LIFETIME_START/LIFETIME_END markers.
Summary

These are pseudos without any users, so DCE was killing them in the combiner.

Marking them as having side effects doesn't seem quite right since they don't.

Gives a nice 0.3% geomean size win on CTMark -Os.

Diff Detail

Event Timeline

aemerson created this revision.Mar 17 2021, 1:29 PM
Herald added subscribers: hiraditya, rovka. · View Herald TranscriptMar 17 2021, 1:29 PM
aemerson requested review of this revision.Mar 17 2021, 1:29 PM
Herald added a subscriber: wdng. · View Herald TranscriptMar 17 2021, 1:29 PM
Harbormaster completed remote builds in B94311: Diff 331356.Mar 17 2021, 2:30 PM
arsenm added inline comments.Mar 17 2021, 3:02 PM
llvm/test/CodeGen/AArch64/GlobalISel/lifetime-marker-no-dce.mir
8–18

Don't need the IR section

aemerson added inline comments.Mar 17 2021, 3:21 PM
llvm/test/CodeGen/AArch64/GlobalISel/lifetime-marker-no-dce.mir
8–18

We do because we need the stack slot defined by the alloca.

arsenm added inline comments.Mar 17 2021, 3:21 PM
llvm/test/CodeGen/AArch64/GlobalISel/lifetime-marker-no-dce.mir
8–18

You don't need it, you just need to drop the name

aemerson updated this revision to Diff 331399.Mar 17 2021, 3:45 PM
Harbormaster completed remote builds in B94342: Diff 331399.Mar 17 2021, 4:34 PM
arsenm accepted this revision.Mar 17 2021, 5:17 PM
This revision is now accepted and ready to land.Mar 17 2021, 5:17 PM
Closed by commit rG28963d895b52: [GlobalISel] Don't DCE LIFETIME_START/LIFETIME_END markers. (authored by aemerson). · Explain WhyMar 17 2021, 6:21 PM
This revision was automatically updated to reflect the committed changes.
aemerson added a commit: rG28963d895b52: [GlobalISel] Don't DCE LIFETIME_START/LIFETIME_END markers..

Revision Contents

PathSize
llvm/
lib/
CodeGen/
GlobalISel/
4 lines
test/
CodeGen/
AArch64/
GlobalISel/
43 lines

Diff 331356

llvm/lib/CodeGen/GlobalISel/Utils.cpp

Show First 20 Lines • Show All 194 Lines • ▼ Show 20 Linesbool llvm::isTriviallyDead(const MachineInstr &MI,
const MachineRegisterInfo &MRI) { const MachineRegisterInfo &MRI) {
// FIXME: This logical is mostly duplicated with // FIXME: This logical is mostly duplicated with
// DeadMachineInstructionElim::isDead. Why is LOCAL_ESCAPE not considered in // DeadMachineInstructionElim::isDead. Why is LOCAL_ESCAPE not considered in
// MachineInstr::isLabel? // MachineInstr::isLabel?
// Don't delete frame allocation labels. // Don't delete frame allocation labels.
if (MI.getOpcode() == TargetOpcode::LOCAL_ESCAPE) if (MI.getOpcode() == TargetOpcode::LOCAL_ESCAPE)
return false; return false;
// LIFETIME markers should be preserved even if they seem dead.
if (MI.getOpcode() == TargetOpcode::LIFETIME_START ||
MI.getOpcode() == TargetOpcode::LIFETIME_END)
return false;
// If we can move an instruction, we can remove it. Otherwise, it has // If we can move an instruction, we can remove it. Otherwise, it has
// a side-effect of some sort. // a side-effect of some sort.
bool SawStore = false; bool SawStore = false;
if (!MI.isSafeToMove(/*AA=*/nullptr, SawStore) && !MI.isPHI()) if (!MI.isSafeToMove(/*AA=*/nullptr, SawStore) && !MI.isPHI())
return false; return false;
// Instructions without side-effects are dead iff they only define dead vregs. // Instructions without side-effects are dead iff they only define dead vregs.
▲ Show 20 Lines • Show All 665 Lines • Show Last 20 Lines

llvm/test/CodeGen/AArch64/GlobalISel/lifetime-marker-no-dce.mir

  • This file was added.
# NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py
# RUN: llc -run-pass=aarch64-prelegalizer-combiner -global-isel -verify-machineinstrs %s -o - | FileCheck %s
# Check that we don't DCE the lifetime markers even though they don't have any users.
--- |
target datalayout = "e-m:e-i8:8:32-i16:16:32-i64:64-i128:128-n32:64-S128"
target triple = "aarch64"
declare void @llvm.lifetime.start.p0i8(i64 immarg, i8* nocapture) #0
declare void @llvm.lifetime.end.p0i8(i64 immarg, i8* nocapture) #0
define void @test_lifetime_no_dce() {
%slot = alloca i8, i32 4, align 4
call void @llvm.lifetime.start.p0i8(i64 0, i8* %slot)
call void @llvm.lifetime.end.p0i8(i64 0, i8* %slot)
ret void
}
attributes #0 = { argmemonly nofree nosync nounwind willreturn }
arsenmUnsubmitted
Not Done
ReplyInline Actions

Don't need the IR section

arsenm: Don't need the IR section
aemersonAuthorUnsubmitted
Done
ReplyInline Actions

We do because we need the stack slot defined by the alloca.

aemerson: We do because we need the stack slot defined by the alloca.
arsenmUnsubmitted
Not Done
ReplyInline Actions

You don't need it, you just need to drop the name

arsenm: You don't need it, you just need to drop the name
...
---
name: test_lifetime_no_dce
alignment: 4
tracksRegLiveness: true
registers:
- { id: 0, class: _ }
frameInfo:
maxAlignment: 4
stack:
- { id: 0, name: slot, size: 4, alignment: 4 }
machineFunctionInfo: {}
body: |
bb.1 (%ir-block.0):
; CHECK-LABEL: name: test_lifetime_no_dce
; CHECK: LIFETIME_START %stack.0.slot
; CHECK: LIFETIME_END %stack.0.slot
; CHECK: RET_ReallyLR
%0:_(p0) = G_FRAME_INDEX %stack.0.slot
LIFETIME_START %stack.0.slot
LIFETIME_END %stack.0.slot
RET_ReallyLR
...