2009/3/18 Dale Johannesen <span dir="ltr"><<a href="mailto:dalej@apple.com">dalej@apple.com</a>></span><br><div class="gmail_quote"><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">
<div style=""><div><div class="im"><div>On Mar 18, 2009, at 11:30 AMPDT, Misha Brukman wrote:</div><blockquote type="cite">Can someone with llvm-gcc/ARM expertise (Dale?) please review Sandeep's patch?  The patch works for me in building an LLVM-based cross-compiler from x86_64/Linux to ARM/Linux.<br>
I have been able to build without the patch to the asm file, but several people have reported needing the asm patch on <a href="http://llvm.org/PR2545" target="_blank">http://llvm.org/PR2545</a> .</blockquote></div><div>Looks OK to me.</div>
</div></div></blockquote><div><br>Thanks for the review.  Sandeep, I've committed your patch (with some
modifications) to arm.h -- please sync and see if this version works
for you.<br> </div><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;"><div style=""><div><div>The asm file is the only thing I'd have reservations about (that isn't marked as a local change, btw, and I don't think it was), but the parentheses should work fine.</div>
</div></div></blockquote><div><br>I haven't yet submitted the .asm patch -- I'm also curious how the /* LLVM LOCAL */ markers work in the Apple merges when it's not an addition but rather a modification, e.g.:<br>
<br style="font-family: courier new,monospace;"></div></div><span style="font-family: courier new,monospace;">-       THUMB_DIV_MOD_BODY 1</span><br style="font-family: courier new,monospace;"><span style="font-family: courier new,monospace;">+       /* LLVM LOCAL */</span><br style="font-family: courier new,monospace;">
<span style="font-family: courier new,monospace;">+       THUMB_DIV_MOD_BODY(1)</span><br style="font-family: courier new,monospace;"><br>Will Apple merger think that this is a new line added in the LLVM and complain that it's missing a line without the parens from the Apple side, and end up inserting both?  I admit I'm still a bit unclear as to how the auto-patcher works in all cases.<br>
<br>Would it make sense to do these fixes on the Apple side of the tree, so that Apple and LLVM are in agreement on this?  It looks like a trivial change.  Or I can make a patch with /* APPLE LOCAL */ markers so that Apple's and public LLVM SVN don't differ so much for such a change.<br>
<br>Let me know what you prefer.<br><br>Misha<br>