> + /// allocateSpace - reserves space in the current block if any, or > + /// allocate a new one of the given size + virtual void > *allocateSpace(intptr_t Size, unsigned Alignment); + > Please capitalize "reserves".ok.> + /// allocateSpace - general-purpose space allocator > Better comments please. :-) Also please end the sentence with a period > or Chris' head will explode. :-)ok, sure, I guess we don't want that :)> unsigned char *result = (unsigned char *)CurBlock+1; > + > + if (Alignment == 0) Alignment = 1; > + result = (unsigned char*)(((intptr_t)result+Alignment-1) & > + ~(intptr_t)(Alignment-1)); > If type of result is intptr_t, you can avoid some of the casting, right?Well, I basically copied that code from the MachineCodeEmitter class :P I must confess that I don't know what the standard says about casting. In the last times I tried guessing (when implementing some parts in clang) I was wrong, so.. :) Apart of fixing these minor things, may I commit the patch? Thanks, Nuno
On Oct 16, 2008, at 11:29 AM, Nuno Lopes wrote:>> + /// allocateSpace - reserves space in the current block if any, or >> + /// allocate a new one of the given size + virtual void >> *allocateSpace(intptr_t Size, unsigned Alignment); + >> Please capitalize "reserves". > > ok. > > >> + /// allocateSpace - general-purpose space allocator >> Better comments please. :-) Also please end the sentence with a >> period >> or Chris' head will explode. :-) > > ok, sure, I guess we don't want that :) > > >> unsigned char *result = (unsigned char *)CurBlock+1; >> + >> + if (Alignment == 0) Alignment = 1; >> + result = (unsigned char*)(((intptr_t)result+Alignment-1) & >> + ~(intptr_t)(Alignment-1)); >> If type of result is intptr_t, you can avoid some of the casting, >> right? > > Well, I basically copied that code from the MachineCodeEmitter > class :P > I must confess that I don't know what the standard says about > casting. In > the last times I tried guessing (when implementing some parts in > clang) I > was wrong, so.. :) > > Apart of fixing these minor things, may I commit the patch?Yes! But please do some sanity testing using the test suite. Thanks, Evan> > > Thanks, > Nuno > > _______________________________________________ > LLVM Developers mailing list > LLVMdev at cs.uiuc.edu http://llvm.cs.uiuc.edu > http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
>>> + /// allocateSpace - reserves space in the current block if any, or >>> + /// allocate a new one of the given size + virtual void >>> *allocateSpace(intptr_t Size, unsigned Alignment); + >>> Please capitalize "reserves". >> >> ok. >> >> >>> + /// allocateSpace - general-purpose space allocator >>> Better comments please. :-) Also please end the sentence with a >>> period >>> or Chris' head will explode. :-) >> >> ok, sure, I guess we don't want that :) >> >> >>> unsigned char *result = (unsigned char *)CurBlock+1; >>> + >>> + if (Alignment == 0) Alignment = 1; >>> + result = (unsigned char*)(((intptr_t)result+Alignment-1) & >>> + ~(intptr_t)(Alignment-1)); >>> If type of result is intptr_t, you can avoid some of the casting, >>> right? >> >> Well, I basically copied that code from the MachineCodeEmitter >> class :P >> I must confess that I don't know what the standard says about >> casting. In >> the last times I tried guessing (when implementing some parts in >> clang) I >> was wrong, so.. :) >> >> Apart of fixing these minor things, may I commit the patch? > > Yes! But please do some sanity testing using the test suite.Ok, patch commited! There are no test regressions and the PHP JIT compiler seems to be working again :) Nuno