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

Eli Friedman eli.friedman at gmail.com
Tue Oct 2 14:51:35 PDT 2012


+  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
>



More information about the llvm-commits mailing list