llvm dev - Mar 2012 - [LLVMdev] apparent mistake in several ports register td file ???

The field Num seems to have no meaning. It is not recognized by the 
backend tools. It does not hurt anything but should not be there.

// We have banks of 32 registers each.
class MipsReg<string n> : Register<n> {
   field bits<5> Num;
   let Namespace = "Mips";
}

class ARMReg<bits<4> num, string n, list<Register> subregs =
[]> :
Register<n> {
   field bits<4> Num;
   let Namespace = "ARM";
   let SubRegs = subregs;
   // All bits of ARM registers with sub-registers are covered by 
sub-registers.
   let CoveredBySubRegs = 1;
}

class ARMFReg<bits<6> num, string n> : Register<n> {
   field bits<6> Num;
   let Namespace = "ARM";
}

class SparcReg<string n> : Register<n> {
   field bits<5> Num;
   let Namespace = "SP";
}




Then subsequently, further derived types copy the mistake.

// Registers are identified with 5-bit ID numbers.
// Ri - 32-bit integer registers
class Ri<bits<5> num, string n> : SparcReg<n> {
   let Num = num;
}
// Rf - 32-bit floating-point registers
class Rf<bits<5> num, string n> : SparcReg<n> {
   let Num = num;
}
// Rd - Slots in the FP register file for 64-bit floating-point values.
class Rd<bits<5> num, string n, list<Register> subregs> :
SparcReg<n> {
   let Num = num;
   let SubRegs = subregs;
   let SubRegIndices = [sub_even, sub_odd];
   let CoveredBySubRegs = 1;
}

......
// Mips CPU Registers
class MipsGPRReg<bits<5> num, string n> : MipsReg<n> {
   let Num = num;
}

At least or Mips, this line seems extraneous. I removed it and and all 
consequential uses of
that (400 changes to MipsRegisterInfo.td) and make check for mips still 
works.
Am running our full test sequence now.

This Mips part of this was copied from the Sparc port. Similar problems 
in other ports.
Seems this has just been copied many times to new ports.

On 03/21/2012 02:58 PM, reed kotler wrote:
> The field Num seems to have no meaning. It is not recognized by the
> backend tools. It does not hurt anything but should not be there.
>
> // We have banks of 32 registers each.
> class MipsReg<string n>  : Register<n>  {
>     field bits<5>  Num;
>     let Namespace = "Mips";
> }
>
> class ARMReg<bits<4>  num, string n, list<Register>  subregs
= []>  :
> Register<n>  {
>     field bits<4>  Num;
>     let Namespace = "ARM";
>     let SubRegs = subregs;
>     // All bits of ARM registers with sub-registers are covered by
> sub-registers.
>     let CoveredBySubRegs = 1;
> }
>
> class ARMFReg<bits<6>  num, string n>  : Register<n>  {
>     field bits<6>  Num;
>     let Namespace = "ARM";
> }
>
> class SparcReg<string n>  : Register<n>  {
>     field bits<5>  Num;
>     let Namespace = "SP";
> }
>
>
>
>
> Then subsequently, further derived types copy the mistake.
>
> // Registers are identified with 5-bit ID numbers.
> // Ri - 32-bit integer registers
> class Ri<bits<5>  num, string n>  : SparcReg<n>  {
>     let Num = num;
> }
> // Rf - 32-bit floating-point registers
> class Rf<bits<5>  num, string n>  : SparcReg<n>  {
>     let Num = num;
> }
> // Rd - Slots in the FP register file for 64-bit floating-point values.
> class Rd<bits<5>  num, string n, list<Register>  subregs>
: SparcReg<n>  {
>     let Num = num;
>     let SubRegs = subregs;
>     let SubRegIndices = [sub_even, sub_odd];
>     let CoveredBySubRegs = 1;
> }
>
> ......
> // Mips CPU Registers
> class MipsGPRReg<bits<5>  num, string n>  : MipsReg<n>  {
>     let Num = num;
> }
> _______________________________________________
> LLVM Developers mailing list
> LLVMdev at cs.uiuc.edu         http://llvm.cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev

These fields are definitely not needed... however.. it seems that this 
might have been a work in progress that was never completed in tablegen.

For the direct object emitter it's useful to put the register number 
encodings in the td files.
These are currently put in the C++ code instead, for example, 
lib/Target/Mips/MCTargetDesc/MipsBaseInfo.h

So I'm going to leave this in for now and file a bug against tablegen.
However, in order to fix this, the Num fields would need to be added to 
the generic register
definition so there would be some code changes to all the ports needed 
to implement this.

On 03/22/2012 06:50 PM, reed kotler wrote:
> At least or Mips, this line seems extraneous. I removed it and and all
> consequential uses of
> that (400 changes to MipsRegisterInfo.td) and make check for mips still
> works.
> Am running our full test sequence now.
>
> This Mips part of this was copied from the Sparc port. Similar problems
> in other ports.
> Seems this has just been copied many times to new ports.
>
> On 03/21/2012 02:58 PM, reed kotler wrote:
>> The field Num seems to have no meaning. It is not recognized by the
>> backend tools. It does not hurt anything but should not be there.
>>
>> // We have banks of 32 registers each.
>> class MipsReg<string n>   : Register<n>   {
>>      field bits<5>   Num;
>>      let Namespace = "Mips";
>> }
>>
>> class ARMReg<bits<4>   num, string n, list<Register>  
subregs = []>   :
>> Register<n>   {
>>      field bits<4>   Num;
>>      let Namespace = "ARM";
>>      let SubRegs = subregs;
>>      // All bits of ARM registers with sub-registers are covered by
>> sub-registers.
>>      let CoveredBySubRegs = 1;
>> }
>>
>> class ARMFReg<bits<6>   num, string n>   :
Register<n>   {
>>      field bits<6>   Num;
>>      let Namespace = "ARM";
>> }
>>
>> class SparcReg<string n>   : Register<n>   {
>>      field bits<5>   Num;
>>      let Namespace = "SP";
>> }
>>
>>
>>
>>
>> Then subsequently, further derived types copy the mistake.
>>
>> // Registers are identified with 5-bit ID numbers.
>> // Ri - 32-bit integer registers
>> class Ri<bits<5>   num, string n>   : SparcReg<n>   {
>>      let Num = num;
>> }
>> // Rf - 32-bit floating-point registers
>> class Rf<bits<5>   num, string n>   : SparcReg<n>   {
>>      let Num = num;
>> }
>> // Rd - Slots in the FP register file for 64-bit floating-point values.
>> class Rd<bits<5>   num, string n, list<Register>  
subregs>   : SparcReg<n>   {
>>      let Num = num;
>>      let SubRegs = subregs;
>>      let SubRegIndices = [sub_even, sub_odd];
>>      let CoveredBySubRegs = 1;
>> }
>>
>> ......
>> // Mips CPU Registers
>> class MipsGPRReg<bits<5>   num, string n>   :
MipsReg<n>   {
>>      let Num = num;
>> }
>> _______________________________________________
>> LLVM Developers mailing list
>> LLVMdev at cs.uiuc.edu         http://llvm.cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
> _______________________________________________
> LLVM Developers mailing list
> LLVMdev at cs.uiuc.edu         http://llvm.cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev

On Mar 21, 2012, at 2:58 PM, reed kotler wrote:
> The field Num seems to have no meaning. It is not recognized by the 
> backend tools. It does not hurt anything but should not be there.
> 
> class ARMReg<bits<4> num, string n, list<Register> subregs =
[]> :
> Register<n> {
>   field bits<4> Num;
>   let Namespace = "ARM";
>   let SubRegs = subregs;
>   // All bits of ARM registers with sub-registers are covered by 
> sub-registers.
>   let CoveredBySubRegs = 1;
> }
You are right, the field is not used currently. Instead, we have large switch
statements like getARMRegisterNumbering().

Actually, I think it might be useful to have TableGen handle the register
encoding numbering, just like it does for DWARF register numbers.

/jakob

-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20120329/96518955/attachment.html>

On 03/29/2012 08:54 AM, Jakob Stoklund Olesen wrote:
>
> On Mar 21, 2012, at 2:58 PM, reed kotler wrote:
>
>> The field Num seems to have no meaning. It is not recognized by the
>> backend tools. It does not hurt anything but should not be there.
>>
>> class ARMReg<bits<4> num, string n, list<Register>
subregs = []> :
>> Register<n> {
>>   field bits<4> Num;
>>   let Namespace = "ARM";
>>   let SubRegs = subregs;
>>   // All bits of ARM registers with sub-registers are covered by
>> sub-registers.
>>   let CoveredBySubRegs = 1;
>> }
>
> You are right, the field is not used currently. Instead, we have large 
> switch statements like getARMRegisterNumbering().
>
> Actually, I think it might be useful to have TableGen handle the 
> register encoding numbering, just like it does for DWARF register numbers.
>
> /jakob
>
Yes, I was thinking to file a bug against tablegen for this.

But then the definition should be in target.td for Num and others should 
just have a "let".

Reed
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20120329/d190cd48/attachment.html>