[PATCH] Teach SimplifyCFG about address spaces

[Quentin Colombet]

  Hi Matt,

  Overall this LGTM but you have to fix the TODO in the test case.
  Moreover, I would like to have your thoughts about the use of DataLayout::getIntPtrType(Type *) versus DataLayout::getIntPtrType(LLVMContext &, unsigned AddrSpace).

  See my inlined comments for more details.

  Cheers,
  Quentin


================
Comment at: lib/Transforms/Utils/SimplifyCFG.cpp:481
@@ +480,3 @@
+      Value *Ptr = PTII->getPointerOperand();
+      if (PTII->getType() == TD->getIntPtrType(Ptr->getType()))
+        CV = Ptr;
----------------
Although this is correct (and equivalent to what I am about to propose), I wonder if it would not be better (when reading the code) to check for a context + address space instead of using the type interface.

Indeed, this would make explicit what we are checking here.

That said, accessing the pointerTy to get the address space will create more code.

My point is I’m fine with this code, I was just wondering if we can make it more readable based on the API documentation. For instance, I had to dig into the implementation of DataLayout to check that this call was doing the same thing as the context + address space call.
Another option may to mention the address space in the documentation of DataLayout::getIntPtrType(Type*).

================
Comment at: lib/Transforms/Utils/SimplifyCFG.cpp:931
@@ -926,3 +930,3 @@
         assert(TD && "Cannot switch on pointer without DataLayout");
-        CV = Builder.CreatePtrToInt(CV, TD->getIntPtrType(CV->getContext()),
+        CV = Builder.CreatePtrToInt(CV, TD->getIntPtrType(CV->getType()),
                                     "magicptr");
----------------
Ditto.

================
Comment at: lib/Transforms/Utils/SimplifyCFG.cpp:2794
@@ -2789,3 +2793,3 @@
     CompVal = Builder.CreatePtrToInt(CompVal,
-                                     TD->getIntPtrType(CompVal->getContext()),
+                                     TD->getIntPtrType(CompVal->getType()),
                                      "magicptr");
----------------
Ditto.

================
Comment at: test/Transforms/SimplifyCFG/switch_create.ll:4
@@ -2,1 +3,3 @@
+
+; TODO: Other tests should also have check lines with datalayout
 
----------------
When do you plan to do that?


http://llvm-reviews.chandlerc.com/D1602