[llvm] [CodeGen] It is safe to ignore internal reads when sinking (PR #88003)
via llvm-commits
llvm-commits at lists.llvm.org
Wed Jul 17 11:28:37 PDT 2024
https://github.com/AtariDreams updated https://github.com/llvm/llvm-project/pull/88003
>From ce5add03e092ade243eb5e8a42a0d2325c284f8b 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, 1 insertion(+), 6 deletions(-)
diff --git a/llvm/lib/CodeGen/MachineSink.cpp b/llvm/lib/CodeGen/MachineSink.cpp
index d782c8b086319..895bc23a2ff7e 100644
--- a/llvm/lib/CodeGen/MachineSink.cpp
+++ b/llvm/lib/CodeGen/MachineSink.cpp
@@ -2026,12 +2026,7 @@ static bool hasRegisterDependency(MachineInstr *MI,
break;
}
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()) {
+ } else if (MO.readsReg()) {
if (!ModifiedRegUnits.available(Reg)) {
HasRegDependency = true;
break;
More information about the llvm-commits
mailing list