From 2150e3a743f8da5322366f7ec23a072c893edc29 Mon Sep 17 00:00:00 2001 From: Hal Finkel Date: Wed, 8 Jan 2014 21:52:02 +0000 Subject: [PATCH] Conservatively handle multiple MMOs in MIsNeedChainEdge MIsNeedChainEdge, which is used by -enable-aa-sched-mi (AA in misched), had an llvm_unreachable when -enable-aa-sched-mi is enabled and we reach an instruction with multiple MMOs. Instead, return a conservative answer. This allows testing -enable-aa-sched-mi on x86. Also, this moves the check above the isUnsafeMemoryObject checks. isUnsafeMemoryObject is currently correct only for instructions with one MMO (as noted in the comment in isUnsafeMemoryObject): // We purposefully do no check for hasOneMemOperand() here // in hope to trigger an assert downstream in order to // finish implementation. The problem with this is that, had the candidate edge passed the "!MIa->mayStore() && !MIb->mayStore()" check, the hoped-for assert would never happen (which could, in theory, lead to incorrect behavior if one of these secondary MMOs was volatile, for example). llvm-svn: 198795 --- llvm/lib/CodeGen/ScheduleDAGInstrs.cpp | 8 ++--- llvm/test/CodeGen/X86/misched-aa-mmos.ll | 37 ++++++++++++++++++++++++ 2 files changed, 41 insertions(+), 4 deletions(-) create mode 100644 llvm/test/CodeGen/X86/misched-aa-mmos.ll diff --git a/llvm/lib/CodeGen/ScheduleDAGInstrs.cpp b/llvm/lib/CodeGen/ScheduleDAGInstrs.cpp index 703ae3d9cc58..65075893e5ab 100644 --- a/llvm/lib/CodeGen/ScheduleDAGInstrs.cpp +++ b/llvm/lib/CodeGen/ScheduleDAGInstrs.cpp @@ -512,6 +512,10 @@ static bool MIsNeedChainEdge(AliasAnalysis *AA, const MachineFrameInfo *MFI, if (MIa == MIb) return false; + // FIXME: Need to handle multiple memory operands to support all targets. + if (!MIa->hasOneMemOperand() || !MIb->hasOneMemOperand()) + return true; + if (isUnsafeMemoryObject(MIa, MFI) || isUnsafeMemoryObject(MIb, MFI)) return true; @@ -527,10 +531,6 @@ static bool MIsNeedChainEdge(AliasAnalysis *AA, const MachineFrameInfo *MFI, MachineMemOperand *MMOa = *MIa->memoperands_begin(); MachineMemOperand *MMOb = *MIb->memoperands_begin(); - // FIXME: Need to handle multiple memory operands to support all targets. - if (!MIa->hasOneMemOperand() || !MIb->hasOneMemOperand()) - llvm_unreachable("Multiple memory operands."); - // The following interface to AA is fashioned after DAGCombiner::isAlias // and operates with MachineMemOperand offset with some important // assumptions: diff --git a/llvm/test/CodeGen/X86/misched-aa-mmos.ll b/llvm/test/CodeGen/X86/misched-aa-mmos.ll new file mode 100644 index 000000000000..343e26f54725 --- /dev/null +++ b/llvm/test/CodeGen/X86/misched-aa-mmos.ll @@ -0,0 +1,37 @@ +; RUN: llc -enable-misched -enable-aa-sched-mi < %s + +; This generates a decw instruction, which has two MMOs, and an alias SU edge +; query involving that instruction. Make sure this does not crash. + +target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128" +target triple = "x86_64-unknown-linux-gnu" + +%s1 = type { i16, i16, i32 } +%c1 = type { %s1*, %u1, i16, i8 } +%u1 = type { i64 } + +declare zeroext i1 @bar(i64*, i32) #5 + +define i32 @foo() #0 align 2 { +entry: + %temp_rhs = alloca %c1, align 8 + br i1 undef, label %if.else56, label %cond.end.i + +cond.end.i: + %significand.i18.i = getelementptr inbounds %c1* %temp_rhs, i64 0, i32 1 + %exponent.i = getelementptr inbounds %c1* %temp_rhs, i64 0, i32 2 + %0 = load i16* %exponent.i, align 8 + %sub.i = add i16 %0, -1 + store i16 %sub.i, i16* %exponent.i, align 8 + %parts.i.i = bitcast %u1* %significand.i18.i to i64** + %1 = load i64** %parts.i.i, align 8 + %call5.i = call zeroext i1 @bar(i64* %1, i32 undef) #1 + unreachable + +if.else56: + unreachable +} + +attributes #0 = { nounwind uwtable } +attributes #1 = { nounwind } +