[LLVMdev] PATCH: Use size reduction -- wave1

[Gabor Greif]

Chris wrote:


> On Mar 26, 2008, at 3:16 PM, Gabor Greif wrote:
> > here comes the patch for the first wave of Use class size reduction.
> >
> > I have split it into 3 files, corresponding to
> > - header changes
> > - implementation changes
> > - applications
>
> nice!
>
> > This at the moment does not contain the description how the
> > size of the Use class will be reduced from 16 to 12 bytes,
> > I am going to send that in a separate patch.
>
> Right, sounds great.
>
> > This wave primarily consists of changes that are needed
> > to allocate objects with a variable number of embedded <Use>s.
>
> Ok.
>
> > Where the number of <Use>s is constant in the lifetime of an object,
> > I preferred to keep the 'new Instr(...)' syntax. Otherwise I have
> > introduced static 'Create(...)' methods, which are used instead  
> of the
> > 'new Instr(...)' construct.
>
> The bad thing about this is that it is inconsistent.  I'd prefer to
> have consistent use of ::Create for all IR classes to make it easier
> to learn the API.  That said, a temporary moment of inconsistency is
> ok: you could commit this part, then switch the remaining classes  
> over.


Yeah, this is a wart, but it is only temporary as you say.
I already have some sed scripts that aid the transition :-)


>
> > These replace the constructors and the
> > constructors become private/protected. The arguments of the 'Create'
> > methods are identical to the corresponding constructors.
> > Essentially at the moment all introduced 'operator new's end up
> > calling
> > '::operator new(size_t)', so there should be no functionality change
> > at all.
> > This will change in subsequent waves.
>
>
> This basically looks good, some thoughts for the future:
>
> 1) Please (eventually) don't make the 'operator new' override be
> public.  I'd actually prefer all memory alloc/dealloc stuff to be done
> privately to the (vmcore) .cpp files, not exposed through the header.

Okay. I guess these relpacement 'operator new' will go away in favor of
Foo::Create methods.

>
> 2) Eventually we'll need to make the dtor private as well.

Okay. So no more 'delete xxx;". But then we must be careful to
never call the Foo::Destroy() method on a NULL ptr.

>
> 3) Make sure that make check and some reasonable subset of llvm-test
> passes with this patch :)

I have never run llvm-test in the past. Is it just checking it out and
following a readme?

>
> Finally, please update llvm-gcc 4.2 as well when you commit this.
> I'll update clang after you commit.

Well, I have patches for both (attached). But I am caught between a  
rock and a
hard place, because Xcode 2.4.1-built llvm-gcc ICEs-out on the
Release llvm (which is needed as per README-LLVM). So I have a hard
time to run those tests. I keep trying. Possibly softlinking the
Release dirs to the Debug ones.

>
> When you commit, please email llvmdev/commits with an email that says
> "API CHANGE" in the subject line with info on how to upgrade out-of-
> tree projects (e.g. vmkit).

Okay, will include the tcsh scripts I have written.

>
> Thanks Gabor!


You are welcome :-)

	Gabor

>
> -Chris
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20080403/b782c0c5/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: gcc.diff
Type: application/octet-stream
Size: 9918 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20080403/b782c0c5/attachment.obj>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20080403/b782c0c5/attachment-0001.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: clang.diff
Type: application/octet-stream
Size: 12656 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20080403/b782c0c5/attachment-0001.obj>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20080403/b782c0c5/attachment-0002.html>