[PATCH] D50977: [TableGen] Examine entire subreg compositions to detect ambiguity

[Krzysztof Parzyszek via Phabricator via llvm-commits]

kparzysz added a comment.

In D50977#1309705 <https://reviews.llvm.org/D50977#1309705>, @uweigand wrote:

> I must have missed the earlier discussion, but I agree with @bjope 's comment earlier that subreg_h32(V0) -> F0S is actually wrong; there should not be any subreg_h32(V0) at all!


`V0` has a subregister `F0D`, which has a subregister index `subreg_h32` mapping `F0D` to `F0S`. TableGen applies the same index with the same result to the super-register, giving `subreg_h32(V0) = F0S`.  This is just how TableGen does it.  In the long term it's probably TableGen that needs to be fixed, but I don't know what impact that will have on other backends.

Here's the original testcase with the comment explaining how the conflict happens. It's less nuanced than the current testcase, but the comment should still apply.

  include "llvm/Target/Target.td"
  
  def TestInstrInfo : InstrInfo {
  }
  
  def Test : Target {
    let InstructionSet = TestInstrInfo;
  }
  
  let Namespace = "Test" in {
    def subreg_h32  : SubRegIndex<32, 32>;
    def subreg_h64  : SubRegIndex<64, 64>;
    def subreg_hh32 : ComposedSubRegIndex<subreg_h64, subreg_h32>;
  }
  
  class TestReg<string n, list<Register> s> : RegisterWithSubRegs<n, s> {
    let Namespace = "Test";
  }
  
  class FPR32<string n> : TestReg<n, []> {
  }
  
  class FPR64<string n, FPR32 high> : TestReg<n, [high]> {
    let SubRegIndices = [subreg_h32];
  }
  
  class FPR128<string n, FPR64 high> : TestReg<n, [high]> {
    let SubRegIndices = [subreg_h64];
  }
  
  def F0S : FPR32<"f0s">;
  def F0D : FPR64<"f0d",  F0S>;
  def F0Q : FPR128<"f0q", F0D>;
  
  // 1. Because of the explicitly defined composition for subreg_hh32,
  //    subreg_hh32.updateComponents will add to subreg_h64 a composition:
  //    subreg_h64+subreg_h32 -> subreg_hh32.
  // 2. computeSubRegs will add to F0Q a subreg F0D (subreg_h64), and then
  //    transitively F0D's subregs (subreg_h32). It will update F0Q's subreg
  //    map that F0S is a subreg of F0Q reachable via subreg_h32.
  // 3. computeComposites will iterate over subreg maps of each register to
  //    determine subregister index compositions. The composition map already
  //    contains the user-defined composition from (1). Because of (2), i.e.
  //    F0Q.subreg_h32 = F0S, which is also F0Q.subreg_h64.subreg_h32, it is
  //    assumed that subreg_h64+subreg_h32 -> subreg_h32. This creates a
  //    conflict and results in a warning.
  
  def FP32  : RegisterClass<"FP32",  [f32],   32,  (add F0S)>;
  def FP64  : RegisterClass<"FP64",  [f64],   64,  (add F0D)>;
  def FP128 : RegisterClass<"FP128", [v2f64], 128, (add F0Q)>;


Repository:
  rL LLVM

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

https://reviews.llvm.org/D50977