Chris Lattner wrote:> On Mon, 31 Mar 2008, Jon Sargeant wrote: >> I'm attaching another round of changes. Please verify that they are correct. > > Applied with edits: > http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20080331/060556.html > > I figured out what your patches don't apply. Something (your web browser, > editor, etc) is stripping trailing whitespace. > > -Chris >Hmm, I realized that the alignment doesn't have a type in front of it (unlike NumElements) so it's reasonably clear that it's a constant. Saying "constant alignment" isn't necessary. Regarding malloc, I think your wording isn't clear enough: "Allocating zero bytes is undefined." My understanding is that an undefined operation is illegal; however, the implementation is not required to diagnose it. Allocating zero bytes is legal; it's just that the result is meaningless. Consider rewording as "Allocating zero bytes is legal, but the result is undefined. The result of a zero-sized allocation is a valid argument for free." Regarding free, I also think your wording isn't clear enough: "If the pointer is null, the result is undefined." The free result is void. Consider rewording as "If the pointer is null, the operation is valid but does not free the pointer." Regarding alloca, please add "The operation is undefined if there is insufficient memory available." Regarding malloc and alloca, I realized that the size is unsigned, so a negative value for NumElements is impossible. I suggest replacing "it is the number of elements allocated" with "it is the UNSIGNED number of elements allocated". Regarding shl, ashr, and lshr, please add "The second argument is interpreted as an unsigned value." Regarding unwind, replace "The 'unwind' intrinsic" with "The 'unwind' instruction" I'm working on another set of changes now. If it's not too much trouble, it would be more convenient for me to post the changes (as I've done in this e-mail) and let you integrate them into LangRef.html. Best Regards, Jon
Jon Sargeant wrote:> Chris Lattner wrote: >> On Mon, 31 Mar 2008, Jon Sargeant wrote: >>> I'm attaching another round of changes. Please verify that they are correct. >> Applied with edits: >> http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20080331/060556.html >> >> I figured out what your patches don't apply. Something (your web browser, >> editor, etc) is stripping trailing whitespace. >> >> -Chris >> > > Hmm, I realized that the alignment doesn't have a type in front of it > (unlike NumElements) so it's reasonably clear that it's a constant. > Saying "constant alignment" isn't necessary. > > Regarding malloc, I think your wording isn't clear enough: "Allocating > zero bytes is undefined." My understanding is that an undefined > operation is illegal; however, the implementation is not required to > diagnose it. Allocating zero bytes is legal; it's just that the result > is meaningless. Consider rewording as "Allocating zero bytes is legal, > but the result is undefined. The result of a zero-sized allocation is a > valid argument for free." > > Regarding free, I also think your wording isn't clear enough: "If the > pointer is null, the result is undefined." The free result is void. > Consider rewording as "If the pointer is null, the operation is valid > but does not free the pointer." > > Regarding alloca, please add "The operation is undefined if there is > insufficient memory available." > > Regarding malloc and alloca, I realized that the size is unsigned, so a > negative value for NumElements is impossible. I suggest replacing "it > is the number of elements allocated" with "it is the UNSIGNED number of > elements allocated". > > Regarding shl, ashr, and lshr, please add "The second argument is > interpreted as an unsigned value." > > Regarding unwind, replace "The 'unwind' intrinsic" with "The 'unwind' > instruction" > > I'm working on another set of changes now. If it's not too much > trouble, it would be more convenient for me to post the changes (as I've > done in this e-mail) and let you integrate them into LangRef.html. > > Best Regards, > JonChris, I'm awaiting your reply. Best Regards, Jon
Jon Sargeant wrote:> Jon Sargeant wrote: >> Chris Lattner wrote: >>> On Mon, 31 Mar 2008, Jon Sargeant wrote: >>>> I'm attaching another round of changes. Please verify that they are correct. >>> Applied with edits: >>> http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20080331/060556.html >>> >>> I figured out what your patches don't apply. Something (your web browser, >>> editor, etc) is stripping trailing whitespace. >>> >>> -Chris >>> >> Hmm, I realized that the alignment doesn't have a type in front of it >> (unlike NumElements) so it's reasonably clear that it's a constant. >> Saying "constant alignment" isn't necessary. >> >> Regarding malloc, I think your wording isn't clear enough: "Allocating >> zero bytes is undefined." My understanding is that an undefined >> operation is illegal; however, the implementation is not required to >> diagnose it. Allocating zero bytes is legal; it's just that the result >> is meaningless. Consider rewording as "Allocating zero bytes is legal, >> but the result is undefined. The result of a zero-sized allocation is a >> valid argument for free." >> >> Regarding free, I also think your wording isn't clear enough: "If the >> pointer is null, the result is undefined." The free result is void. >> Consider rewording as "If the pointer is null, the operation is valid >> but does not free the pointer." >> >> Regarding alloca, please add "The operation is undefined if there is >> insufficient memory available." >> >> Regarding malloc and alloca, I realized that the size is unsigned, so a >> negative value for NumElements is impossible. I suggest replacing "it >> is the number of elements allocated" with "it is the UNSIGNED number of >> elements allocated". >> >> Regarding shl, ashr, and lshr, please add "The second argument is >> interpreted as an unsigned value." >> >> Regarding unwind, replace "The 'unwind' intrinsic" with "The 'unwind' >> instruction" >> >> I'm working on another set of changes now. If it's not too much >> trouble, it would be more convenient for me to post the changes (as I've >> done in this e-mail) and let you integrate them into LangRef.html. >> >> Best Regards, >> Jon > > Chris, I'm awaiting your reply. > > Best Regards, > Jon > _______________________________________________ > LLVM Developers mailing list > LLVMdev at cs.uiuc.edu http://llvm.cs.uiuc.edu > http://lists.cs.uiuc.edu/mailman/listinfo/llvmdevStill waiting.
On Apr 1, 2008, at 6:41 PM, Jon Sargeant wrote:> Chris Lattner wrote: >> On Mon, 31 Mar 2008, Jon Sargeant wrote: >>> I'm attaching another round of changes. Please verify that they >>> are correct. >> >> Applied with edits: >> http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20080331/060556.html >> > Hmm, I realized that the alignment doesn't have a type in front of it > (unlike NumElements) so it's reasonably clear that it's a constant. > Saying "constant alignment" isn't necessary.It doesn't hurt.> Regarding malloc, I think your wording isn't clear enough: "Allocating > zero bytes is undefined." My understanding is that an undefined > operation is illegal; however, the implementation is not required to > diagnose it. Allocating zero bytes is legal; it's just that the > result > is meaningless. Consider rewording as "Allocating zero bytes is > legal, > but the result is undefined. The result of a zero-sized allocation > is a > valid argument for free."Sure, that makes sense.> Regarding free, I also think your wording isn't clear enough: "If the > pointer is null, the result is undefined." The free result is void. > Consider rewording as "If the pointer is null, the operation is valid > but does not free the pointer."It isn't though. free(NULL) could segfault the app, for example.> Regarding alloca, please add "The operation is undefined if there is > insufficient memory available."Makes sense.> Regarding malloc and alloca, I realized that the size is unsigned, > so a > negative value for NumElements is impossible. I suggest replacing "it > is the number of elements allocated" with "it is the UNSIGNED number > of > elements allocated".I'm not sure why this is more clear.> Regarding shl, ashr, and lshr, please add "The second argument is > interpreted as an unsigned value."Seems reasonable. This is redundant with other wording, but is more clear.> Regarding unwind, replace "The 'unwind' intrinsic" with "The 'unwind' > instruction"Done.> I'm working on another set of changes now. If it's not too much > trouble, it would be more convenient for me to post the changes (as > I've > done in this e-mail) and let you integrate them into LangRef.html.It would definitely be easier to integrate in patch form. Here is what I committed: http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20080414/061277.html -Chris
Chris Lattner wrote:> On Apr 1, 2008, at 6:41 PM, Jon Sargeant wrote: >> Regarding free, I also think your wording isn't clear enough: "If the >> pointer is null, the result is undefined." The free result is void. >> Consider rewording as "If the pointer is null, the operation is valid >> but does not free the pointer." > > It isn't though. free(NULL) could segfault the app, for example.That's not compatible with C free(). From 7.20.3.2/2: "The free function causes the space pointed to by ptr to be deallocated, that is, made available for further allocation. If ptr is a null pointer, no action occurs." For whatever reason, I thought that free of null was a normal no-op in LLVM. Nick
Thanks for your reply. Chris Lattner wrote:>> Regarding free, I also think your wording isn't clear enough: "If the >> pointer is null, the result is undefined." The free result is void. >> Consider rewording as "If the pointer is null, the operation is valid >> but does not free the pointer." > > It isn't though. free(NULL) could segfault the app, for example.See Nick's post.>> Regarding malloc and alloca, I realized that the size is unsigned, >> so a >> negative value for NumElements is impossible. I suggest replacing "it >> is the number of elements allocated" with "it is the UNSIGNED number >> of >> elements allocated". > > I'm not sure why this is more clear.The semantics of malloc and alloca depend on whether you interpret NumElements as a signed or unsigned 32-bit integer. For example, if NumElements is 0xffffFFFF, should the instruction fail (because allocating a negative number of elements doesn't make sense), or should the instruction allocate 2^32-1 elements? I don't see any mention of whether NumElements is signed or unsigned in the documentation.>> I'm working on another set of changes now. If it's not too much >> trouble, it would be more convenient for me to post the changes (as >> I've >> done in this e-mail) and let you integrate them into LangRef.html. > > It would definitely be easier to integrate in patch form. Here is > what I committed: > http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20080414/061277.htmlVery well. I'll submit future changes as patches. Best Regards, Jon