[PATCH] D69639: Add support for lowering 32-bit/64-bit pointers

[Reid Kleckner via Phabricator via llvm-commits]

rnk accepted this revision.
rnk added a comment.
This revision is now accepted and ready to land.

lgtm



================
Comment at: llvm/lib/Target/X86/X86.h:156
+namespace X86AS {
+enum : unsigned {
+  GS = 256,
----------------
akhuang wrote:
> RKSimon wrote:
> > use regular X86 namespace and name the enum?
> > ```
> > namespace X86 {
> > enum class AddressSpace : unsigned {
> > ```
> I tried using an enum class but since address spaces are ints it is a bit inconvenient to convert them to enum types. is it better to not have different namespaces?
Most code treats address spaces as unsigned integers, I'd leave it as a regular enum for now.


================
Comment at: llvm/lib/Target/X86/X86ISelLowering.cpp:27638
+  unsigned SrcAS = N->getSrcAddressSpace();
+  unsigned DstAS = N->getDestAddressSpace();
+
----------------
This value will appear to be unused in a build without assertions, which will create warnings. The simplest fix would be to skip the local and move this call into the assert.


================
Comment at: llvm/lib/Target/X86/X86ISelLowering.cpp:28598
+    unsigned SrcAS = CastN->getSrcAddressSpace();
+    unsigned DstAS = CastN->getDestAddressSpace();
+
----------------
ditto


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69639