Skip to content

Commit

Permalink
[GIsel][CombinerHelper] Don't consider debug insts in dominance queri…
Browse files Browse the repository at this point in the history
…es [3/10]

Summary:
This fixes several issues where the presence of debug instructions could
disable certain combines, due to dominance queries finding uses/defs that
don't actually exist.

Reviewers: dsanders, fhahn, paquette, aemerson

Subscribers: hiraditya, arphaman, aprantl, llvm-commits

Tags: #llvm

Differential Revision: https://reviews.llvm.org/D78253
  • Loading branch information
vedantk committed Apr 23, 2020
1 parent 29e70b4 commit cbfa7b5
Show file tree
Hide file tree
Showing 3 changed files with 16 additions and 9 deletions.
4 changes: 2 additions & 2 deletions llvm/include/llvm/CodeGen/GlobalISel/CombinerHelper.h
Expand Up @@ -77,15 +77,15 @@ class CombinerHelper {

/// Returns true if \p DefMI precedes \p UseMI or they are the same
/// instruction. Both must be in the same basic block.
bool isPredecessor(MachineInstr &DefMI, MachineInstr &UseMI);
bool isPredecessor(const MachineInstr &DefMI, const MachineInstr &UseMI);

/// Returns true if \p DefMI dominates \p UseMI. By definition an
/// instruction dominates itself.
///
/// If we haven't been provided with a MachineDominatorTree during
/// construction, this function returns a conservative result that tracks just
/// a single basic block.
bool dominates(MachineInstr &DefMI, MachineInstr &UseMI);
bool dominates(const MachineInstr &DefMI, const MachineInstr &UseMI);

/// If \p MI is extend that consumes the result of a load, try to combine it.
/// Returns true if MI changed.
Expand Down
19 changes: 13 additions & 6 deletions llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp
Expand Up @@ -429,7 +429,7 @@ bool CombinerHelper::matchCombineExtendingLoads(MachineInstr &MI,
? TargetOpcode::G_SEXT
: TargetOpcode::G_ZEXT;
Preferred = {LLT(), PreferredOpcode, nullptr};
for (auto &UseMI : MRI.use_instructions(LoadValue.getReg())) {
for (auto &UseMI : MRI.use_nodbg_instructions(LoadValue.getReg())) {
if (UseMI.getOpcode() == TargetOpcode::G_SEXT ||
UseMI.getOpcode() == TargetOpcode::G_ZEXT ||
UseMI.getOpcode() == TargetOpcode::G_ANYEXT) {
Expand Down Expand Up @@ -560,7 +560,10 @@ void CombinerHelper::applyCombineExtendingLoads(MachineInstr &MI,
Observer.changedInstr(MI);
}

bool CombinerHelper::isPredecessor(MachineInstr &DefMI, MachineInstr &UseMI) {
bool CombinerHelper::isPredecessor(const MachineInstr &DefMI,
const MachineInstr &UseMI) {
assert(!DefMI.isDebugInstr() && !UseMI.isDebugInstr() &&
"shouldn't consider debug uses");
assert(DefMI.getParent() == UseMI.getParent());
if (&DefMI == &UseMI)
return false;
Expand All @@ -573,7 +576,10 @@ bool CombinerHelper::isPredecessor(MachineInstr &DefMI, MachineInstr &UseMI) {
llvm_unreachable("Block must contain instructions");
}

bool CombinerHelper::dominates(MachineInstr &DefMI, MachineInstr &UseMI) {
bool CombinerHelper::dominates(const MachineInstr &DefMI,
const MachineInstr &UseMI) {
assert(!DefMI.isDebugInstr() && !UseMI.isDebugInstr() &&
"shouldn't consider debug uses");
if (MDT)
return MDT->dominates(&DefMI, &UseMI);
else if (DefMI.getParent() != UseMI.getParent())
Expand All @@ -600,7 +606,7 @@ bool CombinerHelper::findPostIndexCandidate(MachineInstr &MI, Register &Addr,

LLVM_DEBUG(dbgs() << "Searching for post-indexing opportunity for: " << MI);

for (auto &Use : MRI.use_instructions(Base)) {
for (auto &Use : MRI.use_nodbg_instructions(Base)) {
if (Use.getOpcode() != TargetOpcode::G_PTR_ADD)
continue;

Expand All @@ -627,7 +633,8 @@ bool CombinerHelper::findPostIndexCandidate(MachineInstr &MI, Register &Addr,
// forming an indexed one.

bool MemOpDominatesAddrUses = true;
for (auto &PtrAddUse : MRI.use_instructions(Use.getOperand(0).getReg())) {
for (auto &PtrAddUse :
MRI.use_nodbg_instructions(Use.getOperand(0).getReg())) {
if (!dominates(MI, PtrAddUse)) {
MemOpDominatesAddrUses = false;
break;
Expand Down Expand Up @@ -700,7 +707,7 @@ bool CombinerHelper::findPreIndexCandidate(MachineInstr &MI, Register &Addr,
// FIXME: check whether all uses of the base pointer are constant PtrAdds.
// That might allow us to end base's liveness here by adjusting the constant.

for (auto &UseMI : MRI.use_instructions(Addr)) {
for (auto &UseMI : MRI.use_nodbg_instructions(Addr)) {
if (!dominates(MI, UseMI)) {
LLVM_DEBUG(dbgs() << " Skipping, does not dominate all addr uses.");
return false;
Expand Down
@@ -1,4 +1,4 @@
; RUN: llc -mtriple=arm64-apple-ios -global-isel -global-isel-abort=1 -verify-machineinstrs -stop-after=aarch64-prelegalizer-combiner -force-legal-indexing %s -o - | FileCheck %s
; RUN: llc -debugify-and-strip-all-safe -mtriple=arm64-apple-ios -global-isel -global-isel-abort=1 -verify-machineinstrs -stop-after=aarch64-prelegalizer-combiner -force-legal-indexing %s -o - | FileCheck %s

define i8* @test_simple_load_pre(i8* %ptr) {
; CHECK-LABEL: name: test_simple_load_pre
Expand Down

0 comments on commit cbfa7b5

Please sign in to comment.