[llvm] [MachineLICM] Do not rely on hasSideEffect when handling impdefs (PR #145379)

[Anatoly Trosinenko via llvm-commits]

================
@@ -585,6 +580,8 @@ void MachineLICMImpl::ProcessMI(MachineInstr *MI, BitVector &RUDefs,
       }
 
       RUDefs.set(Unit);
+      if (MO.isImplicit())
----------------
atrosinenko wrote:

Dropped the special case in 4c29db17a57236be23c864ce0e02db26af405a0f, thank you!

After investigating `HoistRegionPostRA` and `ProcessMI` functions for a while, it feels like the description of `RUDefs` and `RUClobbers` are hardly accurate:
```cpp
  BitVector RUDefs(NumRegUnits);     // RUs defined once in the loop.
  BitVector RUClobbers(NumRegUnits); // RUs defined more than once.
```

These two variables are defined in `HoistRegionPostRA` and used both in `HoistRegionPostRA` and `ProcessMI` by `set`ting and `test`ing bits as well as by mass-`set`tint bits in `applyBitsNotInRegMaskToRegUnitsMask`.

I have an impression that `RUClobbers` is sometimes set while it would be better to first check `RUDefs` (and possibly set corresponding bit there instead), such as when handling `MO.isRegMask()` case a few lines above - it looks harmless, even though slightly pessimistic, to me.

At the same time, `RUDefs` seems to mix two different properties of a register: "the register is live" vs. "we have observed an instruction that defined the register that was initially dead" - while both should transition to "clobbered" state after defining that register one more time, only the latter implies some sort of non-uniformity. Thus, when `HoistRegionPostRA` rejects candidate instructions as follows:
```cpp
    MachineInstr *MI = Candidate.MI;
    for (const MachineOperand &MO : MI->all_uses()) {
      if (!MO.getReg())
        continue;
      for (MCRegUnit Unit : TRI->regunits(MO.getReg())) {
        if (RUDefs.test(Unit) || RUClobbers.test(Unit)) {
          // If it's using a non-loop-invariant register, then it's obviously
          // not safe to hoist.
          Safe = false;
          break;
        }
      }

      if (!Safe)
        break;
    }
```
it looks like rejecting any instructions having any register inputs given that
```cpp
    // Conservatively treat live-in's as an external def.
    // FIXME: That means a reload that're reused in successor block(s) will not
    // be LICM'ed.
    for (const auto &LI : BB->liveins()) {
      for (MCRegUnit Unit : TRI->regunits(LI.PhysReg))
        RUDefs.set(Unit);
    }
```
Quite surprisingly, it is not, because on X86 something like this is possible (here `$rip` is not a live-in):
```
renamable $rcx = MOV64rm $rip, 1, $noreg, target-flags(x86-gotpcrel) @x, $noreg, debug-location !22 :: (load (s64) from got); t.ll:5:7
```

Nevertheless, assuming that the only reason to treat implicit register operands specially is to ignore dead implicit defs for simplicity, it looks harmless to handle any defined register identically, whether it is implicit or explicit.

https://github.com/llvm/llvm-project/pull/145379