Author: Florian Hahn Date: 2020-01-22T20:03:49+01:00 New Revision: 050e1a3c2688f2daf03456cf94dd3ed79e8ebe7f
URL: https://github.com/llvm/llvm-project/commit/050e1a3c2688f2daf03456cf94dd3ed79e8ebe7f DIFF: https://github.com/llvm/llvm-project/commit/050e1a3c2688f2daf03456cf94dd3ed79e8ebe7f.diff LOG: [AArch64] Don't rename registers with pseudo defs in Ld/St opt. If the root def of for renaming is a noop-pseudo instruction like kill, we would end up without a correct def for the renamed register, causing miscompiles. This patch conservatively bails out on any pseudo instruction. This fixes https://bugs.chromium.org/p/chromium/issues/detail?id=1037912#c70 (cherry picked from commit 300997c41a00b705ca10264c15910dd8d691ab75) Added: Modified: llvm/lib/Target/AArch64/AArch64LoadStoreOptimizer.cpp llvm/test/CodeGen/AArch64/stp-opt-with-renaming.mir Removed: ################################################################################ diff --git a/llvm/lib/Target/AArch64/AArch64LoadStoreOptimizer.cpp b/llvm/lib/Target/AArch64/AArch64LoadStoreOptimizer.cpp index 3156bb446963..bc91d628f0b4 100644 --- a/llvm/lib/Target/AArch64/AArch64LoadStoreOptimizer.cpp +++ b/llvm/lib/Target/AArch64/AArch64LoadStoreOptimizer.cpp @@ -1325,6 +1325,19 @@ canRenameUpToDef(MachineInstr &FirstMI, LiveRegUnits &UsedInBetween, // For defs, check if we can rename the first def of RegToRename. if (FoundDef) { + // For some pseudo instructions, we might not generate code in the end + // (e.g. KILL) and we would end up without a correct def for the rename + // register. + // TODO: This might be overly conservative and we could handle those cases + // in multiple ways: + // 1. Insert an extra copy, to materialize the def. + // 2. Skip pseudo-defs until we find an non-pseudo def. + if (MI.isPseudo()) { + LLVM_DEBUG(dbgs() << " Cannot rename pseudo instruction " << MI + << "\n"); + return false; + } + for (auto &MOP : MI.operands()) { if (!MOP.isReg() || !MOP.isDef() || MOP.isDebug() || !MOP.getReg() || !TRI->regsOverlap(MOP.getReg(), RegToRename)) diff --git a/llvm/test/CodeGen/AArch64/stp-opt-with-renaming.mir b/llvm/test/CodeGen/AArch64/stp-opt-with-renaming.mir index 018827772da5..b57e32338f07 100644 --- a/llvm/test/CodeGen/AArch64/stp-opt-with-renaming.mir +++ b/llvm/test/CodeGen/AArch64/stp-opt-with-renaming.mir @@ -469,3 +469,36 @@ body: | RET undef $lr ... +# Make sure we do not rename if pseudo-defs. Noop pseudo instructions like KILL +# may lead to a missing definition of the rename register. +# +# CHECK-LABEL: name: test14_pseudo +# CHECK: bb.0: +# CHECK-NEXT: liveins: $w8, $fp, $w25 +# CHECK: renamable $w8 = KILL killed renamable $w8, implicit-def $x8 +# CHECK-NEXT: STURXi killed renamable $x8, $fp, -40 :: (store 8) +# CHECK-NEXT: $w8 = ORRWrs $wzr, killed $w25, 0, implicit-def $x8 +# CHECK-NEXT: STURXi killed renamable $x8, $fp, -32 :: (store 8) +# CHECK-NEXT: RET undef $lr +# +name: test14_pseudo +alignment: 4 +tracksRegLiveness: true +liveins: + - { reg: '$x0' } + - { reg: '$x1' } + - { reg: '$x8' } +frameInfo: + maxAlignment: 1 + maxCallFrameSize: 0 +machineFunctionInfo: {} +body: | + bb.0: + liveins: $w8, $fp, $w25 + + renamable $w8 = KILL killed renamable $w8, implicit-def $x8 + STURXi killed renamable $x8, $fp, -40 :: (store 8) + $w8 = ORRWrs $wzr, killed $w25, 0, implicit-def $x8 + STURXi killed renamable $x8, $fp, -32 :: (store 8) + RET undef $lr +... _______________________________________________ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
