[llvm-commits] [LLVMdev] Proposal: New IR instruction for casting between address spaces

Tue Oct 2 08:56:56 PDT 2012

Ping!

> -----Original Message-----
> From: Villmow, Micah
> Sent: Friday, September 28, 2012 2:03 PM
> To: Villmow, Micah; Hal Finkel; Eli Friedman
> Cc: Mon Ping Wang; llvm-commits at cs.uiuc.edu
> Subject: RE: [llvm-commits] [LLVMdev] Proposal: New IR instruction for
> casting between address spaces
> 
> Ok, Here is an updated patch that allows the checking of restrictions on
> bitcast between address spaces with different sizes to be detected
> without relying on target data.
> 
> > -----Original Message-----
> > From: llvm-commits-bounces at cs.uiuc.edu [mailto:llvm-commits-
> > bounces at cs.uiuc.edu] On Behalf Of Villmow, Micah
> > Sent: Friday, September 21, 2012 1:02 PM
> > To: Hal Finkel; Eli Friedman
> > Cc: Mon Ping Wang; llvm-commits at cs.uiuc.edu; llvmdev at cs.uiuc.edu
> > Subject: Re: [llvm-commits] [LLVMdev] Proposal: New IR instruction for
> > casting between address spaces
> >
> > Here is an updated patch which moves TargetData out of Target and into
> > Support/VMCore so that there is no circular dependencies.
> >
> > > -----Original Message-----
> > > From: Hal Finkel [mailto:hfinkel at anl.gov]
> > > Sent: Thursday, September 20, 2012 4:02 PM
> > > To: Eli Friedman
> > > Cc: Villmow, Micah; Mon Ping Wang; llvm-commits at cs.uiuc.edu;
> > > llvmdev at cs.uiuc.edu
> > > Subject: Re: [LLVMdev] Proposal: New IR instruction for casting
> > > between address spaces
> > >
> > > On Thu, 20 Sep 2012 15:34:52 -0700
> > > Eli Friedman <eli.friedman at gmail.com> wrote:
> > >
> > > > On Thu, Sep 20, 2012 at 3:30 PM, Villmow, Micah
> > > > <Micah.Villmow at amd.com> wrote:
> > > > > If I don't bring in TargetData, then there is no way for me to
> > > > > verify the address space size in the verifier or in the
> > > > > auto-upgrade mechanisms.
> > > >
> > > > And that's why I didn't like this approach in the first place.
> > >
> > > I don't think that TargetData belongs in Target. TargetData
> > > represents target information that can be known without linking to
> > > the target descriptions, and thus needs to be available outside of
> Target.
> > > Furthermore, TargetData has no dependencies to the rest of Target
> > > (as far as I can tell). Let's move it into VMCore.
> > >
> > >  -Hal
> > >
> > > >
> > > > -Eli
> > > >
> > > > >> -----Original Message-----
> > > > >> From: Eli Friedman [mailto:eli.friedman at gmail.com]
> > > > >> Sent: Thursday, September 20, 2012 2:32 PM
> > > > >> To: Villmow, Micah
> > > > >> Cc: Chris Lattner; Mon Ping Wang; llvm-commits at cs.uiuc.edu;
> > > > >> llvmdev at cs.uiuc.edu
> > > > >> Subject: Re: [LLVMdev] Proposal: New IR instruction for casting
> > > > >> between address spaces
> > > > >>
> > > > >> We can't add a circular dependency between Target and VMCore.
> > > > >>
> > > > >> -Eli
> > > > >>
> > > > >> On Thu, Sep 20, 2012 at 8:21 AM, Villmow, Micah
> > > > >> <Micah.Villmow at amd.com> wrote:
> > > > >> > Ping!
> > > > >> >
> > > > >> >> -----Original Message-----
> > > > >> >> From: Villmow, Micah
> > > > >> >> Sent: Tuesday, September 18, 2012 4:12 PM
> > > > >> >> To: 'Chris Lattner'; 'Mon Ping Wang'
> > > > >> >> Cc: 'llvm-commits at cs.uiuc.edu'; 'llvmdev at cs.uiuc.edu'
> > > > >> >> Subject: RE: [LLVMdev] Proposal: New IR instruction for
> > > > >> >> casting between address spaces
> > > > >> >>
> > > > >> >> Resending since I got an error.
> > > > >> >>
> > > > >> >> > -----Original Message-----
> > > > >> >> > From: Villmow, Micah
> > > > >> >> > Sent: Tuesday, September 18, 2012 4:04 PM
> > > > >> >> > To: Villmow, Micah; 'Chris Lattner'; 'Mon Ping Wang'
> > > > >> >> > Cc: 'llvm-commits at cs.uiuc.edu'; 'llvmdev at cs.uiuc.edu'
> > > > >> >> > Subject: RE: [LLVMdev] Proposal: New IR instruction for
> > > > >> >> > casting between address spaces
> > > > >> >> >
> > > > >> >> > Added a new patch after some feedback. Also make sure all
> > > > >> >> > of the tools/examples build correctly.
> > > > >> >> >
> > > > >> >> > > -----Original Message-----
> > > > >> >> > > From: Villmow, Micah
> > > > >> >> > > Sent: Tuesday, September 18, 2012 11:24 AM
> > > > >> >> > > To: Villmow, Micah; Chris Lattner; Mon Ping Wang
> > > > >> >> > > Cc: llvm-commits at cs.uiuc.edu; llvmdev at cs.uiuc.edu
> > > > >> >> > > Subject: RE: [LLVMdev] Proposal: New IR instruction for
> > > > >> >> > > casting between address spaces
> > > > >> >> > >
> > > > >> >> > > Here is the patch that i've developed that implements
> > > > >> >> > > the below
> > > > >> >> > points.
> > > > >> >> > > The test itself won't work until the target data changes
> > > > >> >> > > are
> > > > >> added.
> > > > >> >> > >
> > > > >> >> > > > -----Original Message-----
> > > > >> >> > > > From: llvmdev-bounces at cs.uiuc.edu
> > > > >> >> > > > [mailto:llvmdev-bounces at cs.uiuc.edu]
> > > > >> >> > > > On Behalf Of Villmow, Micah
> > > > >> >> > > > Sent: Friday, September 14, 2012 8:16 AM
> > > > >> >> > > > To: Chris Lattner; Mon Ping Wang
> > > > >> >> > > > Cc: llvmdev at cs.uiuc.edu Mailing List
> > > > >> >> > > > Subject: Re: [LLVMdev] Proposal: New IR instruction
> > > > >> >> > > > for casting between address spaces
> > > > >> >> > > >
> > > > >> >> > > >
> > > > >> >> > > >
> > > > >> >> > > > > -----Original Message-----
> > > > >> >> > > > > From: Chris Lattner [mailto:clattner at apple.com]
> > > > >> >> > > > > Sent: Thursday, September 13, 2012 11:53 PM
> > > > >> >> > > > > To: Mon Ping Wang
> > > > >> >> > > > > Cc: Villmow, Micah; llvmdev at cs.uiuc.edu Mailing List
> > > > >> >> > > > > Subject: Re: [LLVMdev] Proposal: New IR instruction
> > > > >> >> > > > > for casting between address spaces
> > > > >> >> > > > >
> > > > >> >> > > > >
> > > > >> >> > > > > On Sep 13, 2012, at 5:55 PM, Mon Ping Wang
> > > > >> >> > > > > <monping at apple.com>
> > > > >> >> > > wrote:
> > > > >> >> > > > > >>> The pointer size is target dependent so it seems
> > > > >> >> > > > > >>> strange to choose
> > > > >> >> > > > > an arbitrary size to convert to and from. Are you
> > > > >> >> > > > > making a practical argument that 64b is sufficient
> > > > >> >> > > > > on all machines so all targets can use that?  In
> > > > >> >> > > > > other words, pointers > 64 doesn't make any sense in
> > > > >> >> > > > > terms of the address space? (A pointer to be >
> > > > >> >> > > > > 64 if clients want to use some upper bits to track
> > > > >> >> > > > > some state I
> > > > >> >> guess).
> > > > >> >> > > > > >>>
> > > > >> >> > > > > >>> In terms of the three new instructions, one
> > > > >> >> > > > > >>> could argue that
> > > > >> >> > > > > ptrtoint and intoptr has the same issue or those can
> > > > >> >> > > > > also explode in a similar way.  To use them, this
> > > > >> >> > > > > seems target dependent so unless we really want to
> > > > >> >> > > > > support all the various addressing structures, I
> > > > >> >> > > > > rather not have them.
> > > > >> >> > > > > >>
> > > > >> >> > > > > >> My point is that any producer of this sort of
> > > > >> >> > > > > >> pointer cast is
> > > > >> >> > > > > already necessarily target specific (it is
> > > > >> >> > > > > generating target-specific address space numbers!).
> > > > >> >> > > > > If the front-end knows the address space to use, it
> > > > >> >> > > > > can know a safe integer size to
> > > > >> >> use.
> > > > >> >> > > > > >>
> > > > >> >> > > > > >
> > > > >> >> > > > > > It depends on what the address space is used for.
> > > > >> >> > > > > > If I'm logically
> > > > >> >> > > > > partitioning an address space that overlap my
> > > > >> >> > > > > pointer size may all be the same size so this issue
> > > > >> >> > > > > doesn't come up other than I know the pointer size
> > > > >> >> > > > > are the
> > same.
> > > > >> >> > > > >
> > > > >> >> > > > > Sure, in that case, use bitcast.
> > > > >> >> > > > >
> > > > >> >> > > > > > My understanding is that is becoming an issue
> > > > >> >> > > > > > since a pointer type
> > > > >> >> > > > > size could be different for different address space.
> > > > >> >> > > > > I agree for the case where the pointer size is
> > > > >> >> > > > > address space dependent that the client has to
> > > > >> >> > > > > understand the size and the properties to decide if
> > > > >> >> > > > > they need to do truncation, sign extension or zero
> > > > >> >> extensions.
> > > > >> >> > > > >
> > > > >> >> > > > > Right.
> > > > >> >> > > > >
> > > > >> >> > > > > > This is a problem for auto upgrade as well.
> > > > >> >> > > > > > Today, we have bit cast
> > > > >> >> > > > > between same size pointers for different address
> > space.
> > > > >> >> > > > > We would need to do something special for auto
> > > > >> >> > > > > upgrade here since the proposal is to not allow bit
> > > > >> >> > > > > cast between pointers of different
> > > > >> >> > > address spaces.
> > > > >> >> > > > >
> > > > >> >> > > > > I haven't followed the details of the proposal, but
> > > > >> >> > > > > I think it makes perfect sense to continue using
> > > > >> >> > > > > bitcast for ptr/ptr casts within the same pointer
> > > > >> >> > > > > size.  If you do that, then there is no auto-upgrade
> > > > >> >> > > > > issue: all existing bc files can just be assumed to
> > > > >> >> > > > > have the same pointer size.
> > > > >> >> > > > [Villmow, Micah] So basically we don't need a new IR
> > > > >> >> > > > instructions, but instead
> > > > >> >> > > > 1) bitcasts between pointers of different size is
> > > > >> >> > > > illegal, the proper approach is inttoptr/ptrtoint.
> > > > >> >> > > > 2) bitcasts between pointers of the same size stays
> > legal.
> > > > >> >> > > > 3) No new IR instruction is needed, as converting
> > > > >> >> > > > between pointers of different sizes requires
> > inttoptr/ptrtoint.
> > > > >> >> > > >
> > > > >> >> > > > The only issues are then to update the verifier to
> > > > >> >> > > > assert on bitcasts between pointers of different sizes
> > > > >> >> > > > and add in auto-upgrade of binaries to switch to
> > inttoptr/ptrtoint.
> > > > >> >> > > > By doing this, I then can clear the way for allowing
> > > > >> >> > > > LLVM to support multiple pointer
> > > > >> >> > sizes.
> > > > >> >> > > >
> > > > >> >> > > > Sound good?
> > > > >> >> > > >
> > > > >> >> > > > Micah
> > > > >> >> > > > >
> > > > >> >> > > > > -Chris
> > > > >> >> > > >
> > > > >> >> > > >
> > > > >> >> > > >
> > > > >> >> > > > _______________________________________________
> > > > >> >> > > > LLVM Developers mailing list
> > > > >> >> > > > LLVMdev at cs.uiuc.edu         http://llvm.cs.uiuc.edu
> > > > >> >> > > > http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
> > > > >> >
> > > > >> >
> > > > >> > _______________________________________________
> > > > >> > LLVM Developers mailing list
> > > > >> > LLVMdev at cs.uiuc.edu         http://llvm.cs.uiuc.edu
> > > > >> > http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
> > > > >> >
> > > > >
> > > > >
> > > > _______________________________________________
> > > > LLVM Developers mailing list
> > > > LLVMdev at cs.uiuc.edu         http://llvm.cs.uiuc.edu
> > > > http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
> > >
> > >
> > >
> > > --
> > > Hal Finkel
> > > Postdoctoral Appointee
> > > Leadership Computing Facility
> > > Argonne National Laboratory

-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: bitcast_between_pointer_patch.txt
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20121002/3a595f53/attachment.txt>