[LLVMdev] FW: RFC: Supporting different sized address space arithmetic

[Eli Friedman]

On Fri, Aug 24, 2012 at 4:07 PM, Villmow, Micah <Micah.Villmow at amd.com> wrote:
>
>
>> -----Original Message-----
>> From: Villmow, Micah
>> Sent: Friday, August 24, 2012 2:56 PM
>> To: 'Eli Friedman'
>> Cc: LLVM Developers Mailing List
>> Subject: RE: [LLVMdev] RFC: Supporting different sized address space
>> arithmetic
>>
>> Eli,
>>  There is a patch that implements the beginning what I think is the
>> correct approach to support the backend overloading the size of the
>> pointer for an address space.
>>
>> Does this seem like the right way to do this?
>>
>> I'm guessing I need to handle a few more nodes(IntToPtr/PtrToInt) and
>> atomics, but this already works for what we want to do.

Why do we need the change in lib/CodeGen/Analysis.cpp?  If
TargetLowering::getValueType() doesn't work for arbitrary address
spaces, you should fix it.

I think my earlier comments about getIntPtrConstant still hold:
instead of "DAG.getIntPtrConstant(Offset, addrSpace)", you can just
write "DAG.getConstant(Offset, PtrTy)".

+  EVT NewPtrVT = TLI.getPointerTy(dyn_cast<PointerType>(
+        SV->getType())->getAddressSpace());
+  if (PtrVT != NewPtrVT) {
+    // Check to see if we want to change the size of the pointer
+    // based on the address space and if so extend or truncate the pointer.
+    Ptr = DAG.getSExtOrTrunc(Ptr, getCurDebugLoc(), NewPtrVT);
+    PtrVT = NewPtrVT;
+  }

This situation should be impossible: you're checking whether a pointer
has the same type as itself.  If you're hitting this, it's just a
symptom of a bug elsewhere.  The same applies to other places you
perform this sort of check outside of "bitcast" lowering.


The more broad issue I see here is that we need changes to the IR to
make sure the mid-level optimizers don't break your code;
specifically, using "bitcast" for lossy conversions is unsafe (so you
need a "ptrcast" operation which can perform potentially lossy
conversions), and you need to update TargetData so we can correctly
compute the size of an arbitrary pointer.  I was under the impression
that you were planning to propose a fix for those separately, but it
doesn't look like you have.

-Eli