Cleaning up static initializers in InterferenceCache by moving the NoInterference static object into the class.

[beanz]

Summary

This is a pretty straight forward transformation to change a static member into an instance member.

[chandlerc]

lib/CodeGen/InterferenceCache.h

174

I don't think you should make the Cursor object larger here. One of three options should be employed:

1. If possible, just construct a null temporary BlockInterference object where one is needed, or provide a factor function to do so.
2. Alternatively (perhaps if that fails because it needs to have identity), make this a static *constant* (ie, constant initialized, should be possible from glancing at it). You could even make it a constexpr when we have support for that.
3. If nothing else works, store it in the Entry, and reach it via the CacheEntry pointer.

[beanz]

lib/CodeGen/InterferenceCache.h

174

Unfortunately I don't think any of these options work without significant reworking of the code.

1. We can't construct a null temporary because the way this code works is based on having a pointer to the BlockInterference object.
2. BlockInterference objects can't be statically initialized because they have user defined constructors and contain SlotIndex members. I don't think we can statically initialize those without constexpr.
3. Unfortunately it can't be in the CacheEntry either because the only time this is used is when CacheEntry is null.

Thoughts?

[chandlerc]

lib/CodeGen/InterferenceCache.h

174

Mostly frustration. ;] This code isn't making this easy.

The code clearly isn't *using* the SlotIndex members for the "no" case so I think this is just a badly designed set of data structures. I think you'll have to do the substantial changes here.

I would try to remove the use of a pointer to a special BlockInterference object as the sigil for "no interference". Instead I would try to use a null pointer or a separate flag (potentially embedded into the pointer via PointerUnion).

Sorry, this one just looks like one of the hard ones.

[beanz]

That is a totally reasonable answer. I'll start working on the larger patches.

Thanks,
-Chris

lib/CodeGen/InterferenceCache.h

public:
   /// getMaxCursors - Return the maximum number of concurrent cursors that can
   /// be supported.
   unsigned getMaxCursors() const { return CacheEntries; }
 
   /// Cursor - The primary query interface for the block interference cache.
   class Cursor {
     Entry *CacheEntry;
     BlockInterference *Current;
-    static BlockInterference NoInterference;
+    BlockInterference NoInterference;
 
     void setEntry(Entry *E) {
       Current = nullptr;
       // Update reference counts. Nothing happens when RefCount reaches 0, so
       // we don't have to check for E == CacheEntry etc.
       if (CacheEntry)
         CacheEntry->addRef(-1);
       CacheEntry = E;

lib/CodeGen/InterferenceCache.cpp

 #include "llvm/CodeGen/LiveIntervalAnalysis.h"
 #include "llvm/Support/ErrorHandling.h"
 #include "llvm/Target/TargetRegisterInfo.h"
 
 using namespace llvm;
 
 #define DEBUG_TYPE "regalloc"
 
-// Static member used for null interference cursors.
-InterferenceCache::BlockInterference InterferenceCache::Cursor::NoInterference;
-
 // Initializes PhysRegEntries (instead of a SmallVector, PhysRegEntries is a
 // buffer of size NumPhysRegs to speed up alloc/clear for targets with large
 // reg files). Calloced memory is used for good form, and quites tools like
 // Valgrind too, but zero initialized memory is not required by the algorithm:
 // this is because PhysRegEntries works like a SparseSet and its entries are
 // only valid when there is a corresponding CacheEntries assignment. There is
 // also support for when pass managers are reused for targets with different
 // numbers of PhysRegs: in this case PhysRegEntries is freed and reinitialized.