[PATCH] Teach SimplifyCFG about address spaces
Matt Arsenault
Matthew.Arsenault at amd.com
Wed Oct 16 11:43:29 PDT 2013
================
Comment at: test/Transforms/SimplifyCFG/switch_create.ll:4
@@ -2,1 +3,3 @@
+
+; TODO: Other tests should also have check lines with datalayout
----------------
Quentin Colombet wrote:
> When do you plan to do that?
I've been waiting for my allow multiple prefix FileCheck patch to be committed to fix things like this.
================
Comment at: lib/Transforms/Utils/SimplifyCFG.cpp:481
@@ +480,3 @@
+ Value *Ptr = PTII->getPointerOperand();
+ if (PTII->getType() == TD->getIntPtrType(Ptr->getType()))
+ CV = Ptr;
----------------
Quentin Colombet wrote:
> 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*).
I don't consistently prefer either way. I'll usually use the address space if I already have a variable around with it. Usually the reviews I get say the opposite though.
The only time I think the type way is better is when there could be a vector of pointers, and the type way will do the correct thing. The address space version returns the scalar pointer type.
http://llvm-reviews.chandlerc.com/D1602
More information about the llvm-commits
mailing list