[PATCH] D55287: VirtRegMap: Support partially allocated virtual registers

[Matt Arsenault via Phabricator via llvm-commits]

arsenm closed this revision.
arsenm added a comment.

55a29c6b71c9b80353ccb17c0dd15dde5c9940b3 <https://reviews.llvm.org/rG55a29c6b71c9b80353ccb17c0dd15dde5c9940b3>



================
Comment at: lib/CodeGen/VirtRegMap.cpp:624
+      }
+    }
+  }
----------------
qcolombet wrote:
> arsenm wrote:
> > qcolombet wrote:
> > > Although that's unlikely to happen, we could imagine that another physical reg that share regunits with the one we are clearing is still relevant for subsequence allocation.
> > > 
> > > Put differently, why is this code valid?
> > > 
> > > In theory we may clear RegA's regunits, while they overlap with RegB's regunits and RegB's live interval needs to be valid.
> > For the real use case I have, that can't happen. If these aren't removed, there will be a huge amount of work to maintain these accurately on the off chance this happens. I think this should just be documented as a restriction for the register class filter functions. If one regunit is disabled, any register class that has a register overlapping that regunit also needs to be disabled.
> I was thinking along the same line.
> Could you think of some assert that we could use to make sure this doesn't happen?
I'm not sure what the assert would look like, since you can't generically ask what the register class for a physical register is. In practice you would hit verifier errors after this point. If the filter was a list of register classes rather than a callback, I might be able to do something with that.


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

https://reviews.llvm.org/D55287