[llvm] Question: What is the correct interpretation of LaneBitmask? (PR #109797)

[Sander de Smalen via llvm-commits]

================
@@ -521,26 +521,62 @@ bool TargetRegisterInfo::getCoveringSubRegIndexes(
   SmallVector<unsigned, 8> PossibleIndexes;
   unsigned BestIdx = 0;
   unsigned BestCover = 0;
-
+  unsigned BestSize = 0;
   for (unsigned Idx = 1, E = getNumSubRegIndices(); Idx < E; ++Idx) {
     // Is this index even compatible with the given class?
     if (getSubClassWithSubReg(RC, Idx) != RC)
       continue;
     LaneBitmask SubRegMask = getSubRegIndexLaneMask(Idx);
+
+    // The code below (now disabled) is not correct when the lane mask does not
+    // cover the full sub-register. For example, when 16-bit H0 has only one
+    // subreg B0 of 8-bits, and the high lanes are not defined in TableGen (e.g.
+    // by defining B0 and some artificial B0_HI register), then this function
+    // may return either `bsub` or `hsub`, whereas in this case we'd want it to
+    // return the *largest* (in bithwidth) sub-register index. It is better to
+    // keep looking for the biggest one, but this is rather expensive.
+    //
+    // Before thinking of how to solve this, I first want to ask the
+    // question: What is the meaning of 'LaneBitmask'?
+    //
+    // According to LaneBitmask.h:
+    //
+    //  "The individual bits in a lane mask can't be assigned
+    //   any specific meaning. They can be used to check if two
+    //   sub-register indices overlap."
+    //
+    // However, in the code here the assumption is made that if
+    // 'SubRegMask == LaneMask', then the subregister must overlap *fully*.
+    // This is not the case for the way AArch64 registers are defined
+    // at the moment.
+    //
+    // Which interpretation is correct?
+    //
+    // A) If the meaning is 'if the bits are equal, the sub-registers overlap but
+    //    not necessarily fully', then we should fix the code in this function (in
+    //    a better way than just disabling it).
----------------
sdesmalen-arm wrote:

Thanks for confirming! Now that this is cleared up, I'll close this PR.

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