On 08/08/2013 02:52 PM, Micah Villmow wrote:> This is something I proposed last year, so you might want to read up on that discussion first. > > You can find it here: > http://lists.cs.uiuc.edu/pipermail/llvmdev/2012-September/053277.htmlUhm... I read the discussion, probably I've not understood the problem well: the conclusion (it has been committed?) is that to handle address space conversion that imply a change in the size of pointers must be expressed as a pair ptrtoint/inttoptr. What if the address space conversione logically change also the value of the pointer? Indeed, trying to port that solution in the context of in-progress proposal: how to handle the fact that the addrspace information is logical? Is the frontend that need to produce the correct address space conversion? Why having an opaque instruction/generic-intrinsic that represent the conversion (what the conversion imply is target specific, so that each target should lower this intrinsic using the knowledge of the source and destination address space) is bad? Thanks again. -Michele
It was commited at one point, however due to personal matters, I was not able to respond to issue that arose and the changes were reverted. The longer explanation for why it was reverted can be read here: http://lists.cs.uiuc.edu/pipermail/llvmdev/2012-November/055098.html Micah> -----Original Message----- > From: Michele Scandale [mailto:michele.scandale at gmail.com] > Sent: Thursday, August 08, 2013 7:20 AM > To: Micah Villmow > Cc: Justin Holewinski; LLVM Developers Mailing List > Subject: Re: [LLVMdev] Address space extension > > On 08/08/2013 02:52 PM, Micah Villmow wrote: > > This is something I proposed last year, so you might want to read up on that > discussion first. > > > > You can find it here: > > http://lists.cs.uiuc.edu/pipermail/llvmdev/2012-September/053277.html > > Uhm... I read the discussion, probably I've not understood the problem > well: the conclusion (it has been committed?) is that to handle address space > conversion that imply a change in the size of pointers must be expressed as a > pair ptrtoint/inttoptr. What if the address space conversione logically change > also the value of the pointer? > > Indeed, trying to port that solution in the context of in-progress > proposal: how to handle the fact that the addrspace information is logical? Is > the frontend that need to produce the correct address space conversion? > Why having an opaque instruction/generic-intrinsic that represent the > conversion (what the conversion imply is target specific, so that each target > should lower this intrinsic using the knowledge of the source and destination > address space) is bad? > > Thanks again. > > -Michele
On 8 Aug 2013, at 15:19, Michele Scandale <michele.scandale at gmail.com> wrote:> Uhm... I read the discussion, probably I've not understood the problem well: the conclusion (it has been committed?) is that to handle address space conversion that imply a change in the size of pointers must be expressed as a pair ptrtoint/inttoptr. What if the address space conversione logically change also the value of the pointer?We discussed this in detail at the DevMeeting in San Jose last year, and the consensus was that the required semantics were: - Every cast between pointers in different address spaces should be an addrspacecast instruction - Bitcasts between different address spaces are invalid. - The semantics of inttoptr of inttoptr where the source and destination are in different address spaces are target-dependent and may be undefined for any given conversion As this involves a change to the bitcode format, the auto-upgrader must transform bitcasts and ptrtoint-inttoptr sequences into addrspacecasts. The verifier needed to be modified to reject the now-invalid operations. SelectionDAG would need to grow an ADDRSPACECAST node to handle this correctly. None of these steps was implemented. The optimisers also needed correctly modifying so that they would not introduce the now-invalid sequences, but this would be easier once the verifier could reject them. David
On Aug 8, 2013, at 7:22 , Micah Villmow <micah.villmow at smachines.com> wrote:> It was commited at one point, however due to personal matters, I was not able to respond to issue that arose and the changes were reverted. > The longer explanation for why it was reverted can be read here: > http://lists.cs.uiuc.edu/pipermail/llvmdev/2012-November/055098.htmlActually you never committed that particular part. I found the lost patch on the mailing list and finished it to handle the annoying ConstantExpr casts.
On Aug 8, 2013, at 7:19 , Michele Scandale <michele.scandale at gmail.com> wrote:> On 08/08/2013 02:52 PM, Micah Villmow wrote: >> This is something I proposed last year, so you might want to read up on that discussion first. >> >> You can find it here: >> http://lists.cs.uiuc.edu/pipermail/llvmdev/2012-September/053277.html > > Uhm... I read the discussion, probably I've not understood the problem well: the conclusion (it has been committed?) is that to handle address space conversion that imply a change in the size of pointers must be expressed as a pair ptrtoint/inttoptr. What if the address space conversione logically change also the value of the pointer?I recently committed patches to reject bitcasts if the address space pointers are different sizes, and accept them if they are the same size as was concluded in the thread. I would like to reject all cross address space bitcasts since it would make some things simpler, but when I tried to do that it broke a good number of optimization tests that would be a lot of work to fix. I have the auto upgrade for turning bitcasts into inttoptr/ptrtoint implemented, but it isn't necessary with the current rule that same sized pointer bitcasts are allowed. This would be always necessary if all cross address space bitcasts were disallowed.
On 8 Aug 2013, at 18:34, Matt Arsenault <arsenm2 at gmail.com> wrote:> I have the auto upgrade for turning bitcasts into inttoptr/ptrtoint implemented, but it isn't necessary with the current rule that same sized pointer bitcasts are allowed. This would be always necessary if all cross address space bitcasts were disallowed.The ptrtoint inttoptr dance isn't adequate for this (trust me - it's what we're doing currently in our back end and it's really, really horrible and only mostly works). We currently have a lot of hacks in our branch to stop mid-level optimisers turning these back into bitcasts, or doing annoying (semantics-breaking) sign / zero extends. If you're fixing this, please fix it properly by introducing an address space cast instruction that is exposed as a SelectionDAG node. David