[PATCH] D37052: Add default address space for functions to the data layout (1/3)

[Dylan McKay via Phabricator via llvm-commits]

dylanmckay added inline comments.


================
Comment at: include/llvm/IR/GlobalValue.h:185
   unsigned getAlignment() const;
+  unsigned getAddressSpace() const;
 
----------------
bjope wrote:
> Is this a preparation for some future patch? I can't see that it is used in this patch (and it does not seem to be related to adding support for "P" in DataLayout).
You are right - this is for a future patch. There is no reason why it should be on this patch however.

Moved to D37054,


================
Comment at: lib/IR/DataLayout.cpp:417
 
 bool DataLayout::operator==(const DataLayout &Other) const {
   bool Ret = BigEndian == Other.BigEndian &&
----------------
bjope wrote:
> I'm not exactly sure what the criteria is for this method (or how it is used).
> But maybe the ProgramAddrSpace member should be compared as well?
> 
> PS. I'm trying to compare your solution with the solution we have in our out-of-tree target, and we have something similar to the ProgramAddrSpace member (we call it FunctionPointerAddressSpace and use 'F' instead of 'P'). As it happens we do not compare the FunctionPointerAddressSpace in this operator==, but I guess that is just something we forgot to add.
Good catch, I have added this check


https://reviews.llvm.org/D37052