[llvm] [CodeGen] It is safe to ignore internal reads when sinking (PR #88003)

[via llvm-commits]

https://github.com/AtariDreams created https://github.com/llvm/llvm-project/pull/88003

Consider this:

By definition, a bundle operand has to maintain the exact order of the individual instructions anyway, which means an internal read register is defined in the same bundle, which determines if that register is a dependency, as defines is what tells RegUnits what registers prior to it are available. Because an internal read is defined as a register defined once and then used, the value does not change after use.

Flagging internal reads as dependent makes sense inside a vacuum. However, given that the bundle will ensure internal read dependencies are kept via keeping all instructions in the same order, then we can get away with, assuming the compiler respects the bundle semantics, not flagging the Machine Instruction as having a register dependency. Because the truth is, at worst instructions after the bundle will depend on whatever writes inside the bundle. Instructions outside the bundle do not change just because one of their operands was read prior.

So basically, because bundles are contiguous sequences of instructions, as long as they are kept contiguous, an internal read is safe to ignore because it will not change between the def and the read, and as long as the def does not change, it does not matter where the instructions are, assuming the other operands not defined in the bundle do not also change. Changes in the operands that are not defined in said bundle are not internal reads and are flagged as dependencies anyway.

>From 5c50e1911c8e98dcb88bfb989165d6bfb5e849a7 Mon Sep 17 00:00:00 2001
From: Rose <gfunni234 at gmail.com>
Date: Mon, 8 Apr 2024 11:31:07 -0400
Subject: [PATCH] [CodeGen] It is safe to ignore internal reads when sinking

Consider this:

By definition, a bundle operand has to maintain the exact order of the individual instructions anyway, which means an internal read register is defined in the same bundle, which determines if that register is a dependency, as defines is what tells RegUnits what registers prior to it are available. Because an internal read is defined as a register defined once and then used, the value does not change after use.

Flagging internal reads as dependent makes sense inside a vacuum. However, given that the bundle will ensure internal read dependencies are kept via keeping all instructions in the same order, then we can get away with, assuming the compiler respects the bundle semantics, not flagging the Machine Instruction as having a register dependency. Because the truth is, at worst instructions after the bundle will depend on whatever writes inside the bundle. Instructions outside the bundle do not change just because one of their operands was read prior.

So basically, because bundles are contiguous sequences of instructions, as long as they are kept contiguous, an internal read is safe to ignore because it will not change between the def and the read, and as long as the def does not change, it does not matter where the instructions are, assuming the other operands not defined in the bundle do not also change. Changes in the operands that are not defined in said bundle are not internal reads and are flagged as dependencies anyway.
---
 llvm/lib/CodeGen/MachineSink.cpp | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/llvm/lib/CodeGen/MachineSink.cpp b/llvm/lib/CodeGen/MachineSink.cpp
index c3a1d3759882d8..fea009b7414968 100644
--- a/llvm/lib/CodeGen/MachineSink.cpp
+++ b/llvm/lib/CodeGen/MachineSink.cpp
@@ -1974,11 +1974,8 @@ static bool hasRegisterDependency(MachineInstr *MI,
       }
       DefedRegsInCopy.push_back(Reg);
 
-      // FIXME: instead of isUse(), readsReg() would be a better fix here,
-      // For example, we can ignore modifications in reg with undef. However,
-      // it's not perfectly clear if skipping the internal read is safe in all
-      // other targets.
-    } else if (MO.isUse()) {
+      // Ignore undef uses and internal reads.
+    } else if (MO.readsReg()) {
       if (!ModifiedRegUnits.available(Reg)) {
         HasRegDependency = true;
         break;