[llvm-commits] [llvm] r52945 - in /llvm/trunk: include/llvm/Target/TargetRegisterInfo.h lib/Target/TargetRegisterInfo.cpp utils/TableGen/RegisterInfoEmitter.cpp

[Chris Lattner]

On Jun 30, 2008, at 5:18 PM, Owen Anderson wrote:
> URL: http://llvm.org/viewvc/llvm-project?rev=52945&view=rev
> Log:
> Replace the dynamically computed std::set lookup method for  
> subregisters with a hashtable-based
> version that is computed by tblgen at the time LLVM is compiled.

Nice!

> = 
> = 
> = 
> = 
> = 
> = 
> = 
> = 
> ======================================================================
> --- llvm/trunk/include/llvm/Target/TargetRegisterInfo.h (original)
> +++ llvm/trunk/include/llvm/Target/TargetRegisterInfo.h Mon Jun 30  
> 19:18:52 2008

>
> @@ -410,7 +412,19 @@
>   /// isSubRegister - Returns true if regB is a sub-register of regA.
>   ///
>   bool isSubRegister(unsigned regA, unsigned regB) const {
> +    // SubregHash is a simple quadratically probed hash table.
> +    size_t index = (regA + regB * 37) % SubregHashSize;

Is SubregHashSize always a power of 2?  If so, it would be better to  
write this as:

     size_t index = (regA + regB * 37) & (SubregHashSize-1);

If not, why not? :)


> +    unsigned ProbeAmt = 2;
> +    while (SubregHash[index*2] != 0 &&
> +           SubregHash[index*2+1] != 0) {
>
> +      if (SubregHash[index*2] == regA && SubregHash[index*2+1] ==  
> regB)
> +        return true;
> +
> +      index = (index + ProbeAmt) % SubregHashSize;

Likewise % -> &

> = 
> = 
> = 
> = 
> = 
> = 
> = 
> = 
> ======================================================================
> --- llvm/trunk/utils/TableGen/RegisterInfoEmitter.cpp (original)
> +++ llvm/trunk/utils/TableGen/RegisterInfoEmitter.cpp Mon Jun 30  
> 19:18:52 2008
> @@ -462,6 +462,70 @@
>                      RegisterAliases);
>     }
>   }
> +
> +  // Print the SubregHashTable, a simple quadratically probed
> +  // hash table for determining if a register is a subregister
> +  // of another register.
> +  unsigned SubregHashTableSize = NextPowerOf2(2 * Regs.size());
> +  unsigned* SubregHashTable =
> +                  (unsigned*)malloc(2 * SubregHashTableSize *  
> sizeof(unsigned));

Please use new[]/delete[] instead of malloc/free.  Does this really  
guarantee that the hash table is sparse enough?  If I had 31  
registers, would this make a table of size 32?  For a probed hash  
table like this, you want at least 30-40% sparsity.

> +  for (unsigned i = 0; i < SubregHashTableSize * 2; ++i)
> +    SubregHashTable[i] = ~0U;

memset?

-Chris