From 52e6d52f10dcc2c7750f8c37d2a408219bda611b Mon Sep 17 00:00:00 2001 From: Amara Emerson Date: Fri, 2 Aug 2019 23:33:13 +0000 Subject: [PATCH] [GlobalISel] Check LLT size matches memory size for non-truncating stores. This was causing a bug where non-truncating stores would be selected instead of truncating ones. Differential Revision: https://reviews.llvm.org/D64845 llvm-svn: 367737 --- llvm/test/TableGen/address-space-patfrags.td | 17 ++++++++++++++++- llvm/utils/TableGen/GlobalISelEmitter.cpp | 18 +++++++++++++----- 2 files changed, 29 insertions(+), 6 deletions(-) diff --git a/llvm/test/TableGen/address-space-patfrags.td b/llvm/test/TableGen/address-space-patfrags.td index 82f924bcb180..cf31294bfe09 100644 --- a/llvm/test/TableGen/address-space-patfrags.td +++ b/llvm/test/TableGen/address-space-patfrags.td @@ -44,6 +44,11 @@ def inst_c : Instruction { let InOperandList = (ins GPR32:$src0, GPR32:$src1); } +def inst_d : Instruction { + let OutOperandList = (outs); + let InOperandList = (ins GPR32:$src0, GPR32:$src1); +} + // SDAG: case 2: { // SDAG-NEXT: // Predicate_pat_frag_b // SDAG-NEXT: SDNode *N = Node; @@ -115,10 +120,20 @@ def : Pat < (inst_c GPR32:$src0, GPR32:$src1) >; -// Test truncstore with specific MemoryVT +// Test non-truncstore has a size equal to LLT check. // GISEL: GIM_Try, /*On fail goto*//*Label 3*/ {{[0-9]+}}, // Rule ID 3 // // GISEL-NEXT: GIM_CheckNumOperands, /*MI*/0, /*Expected*/2, // GISEL-NEXT: GIM_CheckOpcode, /*MI*/0, TargetOpcode::G_STORE, +// GISEL-NEXT: GIM_CheckMemorySizeEqualToLLT, /*MI*/0, /*MMO*/0, /*OpIdx*/0, +def : Pat < + (store GPR32:$src0, GPR32:$src1), + (inst_d GPR32:$src0, GPR32:$src1) +>; + +// Test truncstore with specific MemoryVT +// GISEL: GIM_Try, /*On fail goto*//*Label 4*/ {{[0-9]+}}, // Rule ID 4 // +// GISEL-NEXT: GIM_CheckNumOperands, /*MI*/0, /*Expected*/2, +// GISEL-NEXT: GIM_CheckOpcode, /*MI*/0, TargetOpcode::G_STORE, // GISEL-NEXT: GIM_CheckMemorySizeLessThanLLT, /*MI*/0, /*MMO*/0, /*OpIdx*/0, // GISEL-NEXT: GIM_CheckMemoryAddressSpace, /*MI*/0, /*MMO*/0, /*NumAddrSpace*/2, /*AddrSpace*/123, /*AddrSpace*/455, // GISEL-NEXT: GIM_CheckMemorySizeEqualTo, /*MI*/0, /*MMO*/0, /*Size*/2, diff --git a/llvm/utils/TableGen/GlobalISelEmitter.cpp b/llvm/utils/TableGen/GlobalISelEmitter.cpp index 7e17dafc832d..06cdfd4ab597 100644 --- a/llvm/utils/TableGen/GlobalISelEmitter.cpp +++ b/llvm/utils/TableGen/GlobalISelEmitter.cpp @@ -3347,11 +3347,19 @@ Expected GlobalISelEmitter::createAndImportSelDAGMatcher( continue; } - if (Predicate.isStore() && Predicate.isTruncStore()) { - // FIXME: If MemoryVT is set, we end up with 2 checks for the MMO size. - InsnMatcher.addPredicate( - 0, MemoryVsLLTSizePredicateMatcher::LessThan, 0); - continue; + if (Predicate.isStore()) { + if (Predicate.isTruncStore()) { + // FIXME: If MemoryVT is set, we end up with 2 checks for the MMO size. + InsnMatcher.addPredicate( + 0, MemoryVsLLTSizePredicateMatcher::LessThan, 0); + continue; + } + if (Predicate.isNonTruncStore()) { + // We need to check the sizes match here otherwise we could incorrectly + // match truncating stores with non-truncating ones. + InsnMatcher.addPredicate( + 0, MemoryVsLLTSizePredicateMatcher::EqualTo, 0); + } } // No check required. We already did it by swapping the opcode.