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

Tue Oct 2 16:06:20 PDT 2012

At this point I believe the assumption is that the data layout is valid as error checking should be at TargetData creation and not when the DataLayout string is emitted.

I'll make sure the pointers are printed out in sorted order and submit another patch.

> -----Original Message-----
> From: Eli Friedman [mailto:eli.friedman at gmail.com]
> Sent: Tuesday, October 02, 2012 2:52 PM
> To: Villmow, Micah
> Cc: llvm-commits at cs.uiuc.edu LLVM
> Subject: Re: [LLVMdev] FW: RFC: Supporting different sized address space
> arithmetic
> 
> +  for (DenseMap<unsigned, PointerAlignElem>::const_iterator
> +      pib = Pointers.begin(), pie = Pointers.end();
> +      pib != pie; ++pib) {
> +    const PointerAlignElem &PI = pib->second;
> +    OS << "-p";
> +    if (PI.AddressSpace) {
> +      OS << PI.AddressSpace;
> +    }
> +     OS << ":" << PI.TypeBitWidth*8 << ':' << PI.ABIAlign*8
> +        << ':' << PI.PrefAlign*8;
> 
> Don't iterate over a DenseMap like this; the iteration order is
> unspecified.
> 
> How does the error checking for invalid data layouts work?  It seems
> like there's some missing checks here.
> 
> Otherwise, looks fine.
> 
> -Eli
> 
> On Tue, Oct 2, 2012 at 9:57 AM, Villmow, Micah <Micah.Villmow at amd.com>
> wrote:
> > Ping!
> >
> >> -----Original Message-----
> >> From: Villmow, Micah
> >> Sent: Thursday, September 27, 2012 9:52 AM
> >> To: Villmow, Micah; Eli Friedman
> >> Cc: LLVM Developers Mail
> >> Subject: RE: [LLVMdev] FW: RFC: Supporting different sized address
> >> space arithmetic
> >>
> >> Since the TargetData/Bitcast issue is on hold pending an LLVM dev
> >> meeting BOF it seems, I am revisiting this patch.
> >>
> >> This should have no functional changes, but only allow LLVM to
> >> understand different pointer sizes for the backends that wish to use
> it.
> >>
> >> Micah
> >>
> >> > -----Original Message-----
> >> > From: llvmdev-bounces at cs.uiuc.edu
> >> > [mailto:llvmdev-bounces at cs.uiuc.edu]
> >> > On Behalf Of Villmow, Micah
> >> > Sent: Thursday, September 06, 2012 9:19 AM
> >> > To: Eli Friedman
> >> > Cc: LLVM Developers Mail
> >> > Subject: Re: [LLVMdev] FW: RFC: Supporting different sized address
> >> > space arithmetic
> >> >
> >> > Doh! hit send too soon, patch attached.
> >> >
> >> > > -----Original Message-----
> >> > > From: Villmow, Micah
> >> > > Sent: Thursday, September 06, 2012 9:17 AM
> >> > > To: 'Eli Friedman'
> >> > > Cc: LLVM Developers Mail
> >> > > Subject: RE: [LLVMdev] FW: RFC: Supporting different sized
> >> > > address space arithmetic
> >> > >
> >> > > Eli,
> >> > >  Here is the first of many patches that adds support for
> >> > > specifying different pointer sizes for different address spaces.
> >> > > This is only the modifications to TargetData and not all the
> >> > > changes to the backends/optimizers. There should be no functional
> >> > > changes here since the default value is what the current value
> is.
> >> > >
> >> > > After this is approved, my goal is the following:
> >> > > 1) Add a few interfaces to various functions that simplify
> >> > > retrieving address space information.
> >> > > 2) Update all of the optimizations to use address space
> >> > > information when querying the pointer type.
> >> > > 3) Update the backends to follow suite
> >> > > 4) Update the clients(clang, etc..) to use the correct API.
> >> > > 5) remove the default arguments so that future users must
> >> > > explicitly specify an address space.
> >> > >
> >> > > I'm not sure how to add tests for this since no backend uses it
> >> > > yet, but i'll try to figure something out.
> >> > > Micah
> >> > > > -----Original Message-----
> >> > > > From: Eli Friedman [mailto:eli.friedman at gmail.com]
> >> > > > Sent: Thursday, August 30, 2012 3:43 PM
> >> > > > To: Villmow, Micah
> >> > > > Cc: LLVM Developers Mail
> >> > > > Subject: Re: [LLVMdev] FW: RFC: Supporting different sized
> >> > > > address space arithmetic
> >> > > >
> >> > > > On Thu, Aug 30, 2012 at 3:27 PM, Villmow, Micah
> >> > > > <Micah.Villmow at amd.com>
> >> > > > wrote:
> >> > > > >
> >> > > > >
> >> > > > >> -----Original Message-----
> >> > > > >> From: Eli Friedman [mailto:eli.friedman at gmail.com]
> >> > > > >> Sent: Thursday, August 30, 2012 3:03 PM
> >> > > > >> To: Villmow, Micah
> >> > > > >> Cc: LLVM Developers Mail
> >> > > > >> Subject: Re: [LLVMdev] FW: RFC: Supporting different sized
> >> > > > >> address space arithmetic
> >> > > > >>
> >> > > > >> On Thu, Aug 30, 2012 at 2:38 PM, Villmow, Micah
> >> > > > >> <Micah.Villmow at amd.com>
> >> > > > >> wrote:
> >> > > > >> > Eli,
> >> > > > >> >  Here is an updated patch. This is a lot smaller based on
> >> > > > >> > your
> >> > > > >> feedback and still solves the same problem.
> >> > > > >>
> >> > > > >> The patch appears to be corrupt; can you regenerate it?
> >> > > > > [Villmow, Micah] Attached.
> >> > > > >>
> >> > > > >> > For your comment on the IR changes, I'm reluctant to
> >> > > > >> > introduce changes
> >> > > > >> there because really the backend is overriding the default
> >> > > > >> behavior at a device specific level. The optimizations
> >> > > > >> themselves can be dangerous, but still should produce
> >> > > > >> correct results, this only allows the backend to optimize
> >> > > > >> certain cases
> >> and is opt-in.
> >> > > > >>
> >> > > > >> Suppose I have an array of ten pointers into some
> >> > > > >> address-space which uses 16-bit pointers, and the default
> >> > > > >> pointer size is 64 bits.  How many bytes in memory does that
> >> > > > >> take?  To me, it seems like the obvious answer is 20 bytes,
> >> > > > >> but if you compute it using our current TargetData, you'll
> >> > > > >> come up with an answer
> >> of 80.
> >> > > > >> That
> >> > > can't work.
> >> > > > >>
> >> > > > >> If your answer is that it should be 80, and the size of a
> >> > > > >> pointer isn't something the frontend/IR optimizers should be
> >> > > > >> aware of, I'm not sure your approach makes sense; you could
> >> > > > >> just introduce custom load/store nodes in your target which
> >> > > > >> truncate the pointer, and you wouldn't need to mess with the
> >> > > > >> size of a pointer
> >> > at all.
> >> > > > > [Villmow, Micah] Yeah I see your point here. I don't deal
> >> > > > > with array
> >> > > > of pointers in OpenCL, so didn't think of this case.
> >> > > > >
> >> > > > > So, what about extending data layout to support something
> like:
> >> > > > > p#:size:abi:pref <- specify the size, abi and preference for
> >> > > > > the
> >> > > > pointer of address space '#'. Default behavior is
> p:size:abi:pref.
> >> > > >
> >> > > > That's fine.
> >> > > >
> >> > > > (You'll also need to deal with the fact that LLVM assumes bit
> >> > > > casts across address-spaces are lossless.)
> >> > > >
> >> > > > -Eli
> >