llvm dev - Aug 2013 - [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

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.html
Actually 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