[PATCH] D111566: [SYCL] Fix function pointer address space

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.


================
Comment at: clang/include/clang/Basic/TargetInfo.h:763
+  /// Set the address space for functions for the given target.
+  virtual void setProgramAddressSpace() { ProgramAddrSpace = 0; }
+
----------------
eandrews wrote:
> I'm not entirely sure about setting the default value to zero here. I set it to 0 here based on default value for ProgramAddrSpace  in DataLayout class (https://llvm.org/doxygen/DataLayout_8cpp_source.html#l00186)
> 
> Targets with non-zero value should override this function. I do not know what these targets are (if any). I ran check-llvm and nothing failed. 
I don't think you need this method; targets that want to change the program address space can just set it during construction, since it's a protected field.  I know there are some existing places that use this virtual `set` pattern, but there doesn't seem to be a good reason for it.

Looks like AVR sets the program address space to 1.  (I did a regexp search for `/["-]P[0-9]/` in `lib/Basic/`.)


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

https://reviews.llvm.org/D111566