<html><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; ">Thanks Nuno and Dale. Some comments:<div><br></div><div><pre><font class="Apple-style-span" face="Helvetica" size="3"><span class="Apple-style-span" style="font-size: 12px; white-space: normal;">+ /// allocateSpace - reserves space in the current block if any, or</span></font></pre><pre><font class="Apple-style-span" face="Helvetica" size="3"><span class="Apple-style-span" style="font-size: 12px; white-space: normal;">+ /// allocate a new one of the given size
+ virtual void *allocateSpace(intptr_t Size, unsigned Alignment);
+</span></font></pre><pre><font class="Apple-style-span" face="Helvetica" size="3"><span class="Apple-style-span" style="font-size: 12px; white-space: normal;">Please capitalize "reserves".</span></font></pre><pre><font class="Apple-style-span" face="Helvetica" size="3"><span class="Apple-style-span" style="font-size: 12px; white-space: normal;"><pre><font class="Apple-style-span" face="Helvetica"><span class="Apple-style-span" style="white-space: normal;">+ /// allocateSpace - general-purpose space allocator</span></font></pre></span></font></pre><pre><font class="Apple-style-span" face="Helvetica" size="3"><span class="Apple-style-span" style="font-size: 12px; white-space: normal;">Better comments please. :-) Also please end the sentence with a period or Chris' head will explode. :-)</span></font></pre><pre><pre><font class="Apple-style-span" face="Helvetica" size="3"><span class="Apple-style-span" style="font-size: 12px; white-space: normal;"> unsigned char *result = (unsigned char *)CurBlock+1;</span></font></pre><pre><font class="Apple-style-span" face="Helvetica" size="3"><span class="Apple-style-span" style="font-size: 12px; white-space: normal;"> +</span></font></pre><pre><font class="Apple-style-span" face="Helvetica" size="3"><span class="Apple-style-span" style="font-size: 12px; white-space: normal;"> + if (Alignment == 0) Alignment = 1;</span></font></pre><pre><font class="Apple-style-span" face="Helvetica" size="3"><span class="Apple-style-span" style="font-size: 12px; white-space: normal;"><pre><font class="Apple-style-span" face="Helvetica"><span class="Apple-style-span" style="white-space: normal;">+ result = (unsigned char*)(((intptr_t)result+Alignment-1) &</span></font></pre><pre><font class="Apple-style-span" face="Helvetica"><span class="Apple-style-span" style="white-space: normal;">+ ~(intptr_t)(Alignment-1));</span></font></pre></span></font></pre><pre><font class="Apple-style-span" face="Helvetica" size="3"><span class="Apple-style-span" style="font-size: 12px; white-space: normal;">If type of result is intptr_t, you can avoid some of the casting, right?</span></font></pre><pre><font class="Apple-style-span" face="Helvetica" size="3"><span class="Apple-style-span" style="font-size: 12px; white-space: normal;">Thanks!</span></font></pre><pre><font class="Apple-style-span" face="Helvetica" size="3"><span class="Apple-style-span" style="font-size: 12px; white-space: normal;">Evan</span></font></pre></pre><div><div>On Oct 16, 2008, at 8:45 AM, Nuno Lopes wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite"><div><blockquote type="cite"><blockquote type="cite">Ok, thanks for the explanation. So my first patch doesn't work.<br></blockquote></blockquote><blockquote type="cite"><blockquote type="cite">Also, to be<br></blockquote></blockquote><blockquote type="cite"><blockquote type="cite">clear, this bug has nothing to do with overflowing the JIT memory<br></blockquote></blockquote><blockquote type="cite"><blockquote type="cite">buffer.<br></blockquote></blockquote><blockquote type="cite"><blockquote type="cite">I made another one that takes keeps the allocation of global<br></blockquote></blockquote><blockquote type="cite"><blockquote type="cite">variables in<br></blockquote></blockquote><blockquote type="cite"><blockquote type="cite">the JIT buffer, but it creates a new mem block if it doesn't exist<br></blockquote></blockquote><blockquote type="cite"><blockquote type="cite">(i.e.<br></blockquote></blockquote><blockquote type="cite"><blockquote type="cite">when dumping a global variable out of the scope of a function<br></blockquote></blockquote><blockquote type="cite"><blockquote type="cite">compilation).<br></blockquote></blockquote><blockquote type="cite"><blockquote type="cite">The patch is at:<br></blockquote></blockquote><blockquote type="cite"><blockquote type="cite"><a href="http://web.ist.utl.pt/nuno.lopes/llvm_jit_global_emitter2.txt">http://web.ist.utl.pt/nuno.lopes/llvm_jit_global_emitter2.txt</a><br></blockquote></blockquote><blockquote type="cite"><br></blockquote><blockquote type="cite">OK, I think I finally understand this: the allocation in<br></blockquote><blockquote type="cite">JIT::getOrEmitGlobalVariable doesn't call into JITEmitter but uses the<br></blockquote><blockquote type="cite">MachineCodeEmitter directly. Thus, if you call it from outside the<br></blockquote><blockquote type="cite">JITEmitter, it won't update the data structures the JITEmitter relies<br></blockquote><blockquote type="cite">on. In the usual case, the JITEmitter calls into JIT and updates its<br></blockquote><blockquote type="cite">own data structures, so that case works fine.<br></blockquote><br>exactly! :) Thank you for summarizing. I hope others can now follow this <br>description.<br><br><br><blockquote type="cite">I think adding a smarter allocateSpace to JITEmitter is the right<br></blockquote><blockquote type="cite">general idea, and your patch looks OK. I'd like to get another pair<br></blockquote><blockquote type="cite">of eyes on it though, as I find this area not straightfoward:) Please<br></blockquote><blockquote type="cite">run the testsuite before committing.<br></blockquote><br>Yes, I would also appreciate more reviewers. The JIT's code is particularly <br>complex and I'm also quite new to the LLVM code..<br><br>Thanks,<br>Nuno <br><br>_______________________________________________<br>LLVM Developers mailing list<br><a href="mailto:LLVMdev@cs.uiuc.edu">LLVMdev@cs.uiuc.edu</a> <a href="http://llvm.cs.uiuc.edu">http://llvm.cs.uiuc.edu</a><br><a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev">http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev</a><br></div></blockquote></div><br></div></body></html>