[PATCH] D77029: Fix out-of-bounds BitVector access in RegScavenger

[Matthias Braun via Phabricator via llvm-commits]

MatzeB added a comment.

Ugh... I suspect we're not seeing a problem in practice since missing "kills" just means missed opportunities but it's not a correctness problem.

If we don't want the risk of changing existing behavior then we could replace the whole loop with a TODO comment for now.

BTW: If you are changing the BitVector class anyway, would it be possible to remove the `clear` function (or change its behavior). Given that most users think of a BitVector more as an integer set (and not like a vector that you push_back() to) the behavior can confuse fast...



================
Comment at: llvm/lib/CodeGen/RegisterScavenging.cpp:120-134
   for (const MachineOperand &MO : MI.operands()) {
     if (MO.isRegMask()) {
-      TmpRegUnits.clear();
+      TmpRegUnits.reset();
       for (unsigned RU = 0, RUEnd = TRI->getNumRegUnits(); RU != RUEnd; ++RU) {
         for (MCRegUnitRootIterator RURI(RU, TRI); RURI.isValid(); ++RURI) {
           if (MO.clobbersPhysReg(*RURI)) {
             TmpRegUnits.set(RU);
----------------



================
Comment at: llvm/lib/CodeGen/RegisterScavenging.cpp:120-134
   for (const MachineOperand &MO : MI.operands()) {
     if (MO.isRegMask()) {
-      TmpRegUnits.clear();
+      TmpRegUnits.reset();
       for (unsigned RU = 0, RUEnd = TRI->getNumRegUnits(); RU != RUEnd; ++RU) {
         for (MCRegUnitRootIterator RURI(RU, TRI); RURI.isValid(); ++RURI) {
           if (MO.clobbersPhysReg(*RURI)) {
             TmpRegUnits.set(RU);
----------------
MatzeB wrote:
> 
Given that this appears to do nothing, let's go for this:


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D77029/new/

https://reviews.llvm.org/D77029