I'm still catching up after a bit of time off, but for clarification, I'm actually fine with them being in a single thread. However, I'm likely to review them one at a time within the thread.<div><br></div><div>
Feel free to keep them all in this thread if you'd rather, I'll be trying to get to these through out the week.</div><div class="gmail_extra"><br><br><div class="gmail_quote">On Mon, Jul 30, 2012 at 4:29 PM, Sean Silva <span dir="ltr"><<a href="mailto:silvas@purdue.edu" target="_blank">silvas@purdue.edu</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">[Chandler]<br>
<div class="im">> Can you split this into separate patches and post them individually for review?<br>
<br>
</div>The way I understand "post them individually for review" is that I<br>
think he meant to have a separate email for each one, so that each<br>
patch can get a separate thread for discussion. This makes the review<br>
process a lot easier and more focused.<br>
<br>
I wouldn't send 6 separate emails all at once though. Just send the<br>
first patch in an email. Once that one has been reviewed and<br>
committed, send the second one, etc. It's possible that the review for<br>
the one patch will give you good ideas for improving the other ones,<br>
so it is best if the reviewers only have to point things out once.<br>
Also, since the patches depend on each other, review for one patch may<br>
force the other patches to change.<br>
<span class="HOEnZb"><font color="#888888"><br>
--Sean Silva<br>
</font></span><div class="HOEnZb"><div class="h5"><br>
On Mon, Jul 30, 2012 at 4:02 PM, Michael Liao <<a href="mailto:michael.liao@intel.com">michael.liao@intel.com</a>> wrote:<br>
> Hi,<br>
><br>
> Not sure have any one got chance to review this?<br>
><br>
> Yours<br>
> - Michael<br>
><br>
> On Thu, 2012-07-26 at 02:26 -0700, Michael Liao wrote:<br>
>> On Wed, 2012-07-25 at 15:34 -0700, Chandler Carruth wrote:<br>
>> > First off, these sound like fantastic enhancements! =] Thanks for<br>
>> > pulling them out.<br>
>> ><br>
>> ><br>
>> > Can you split this into separate patches and post them individually<br>
>> > for review? I think each of your bullet points would make for a good<br>
>> > patch to review.<br>
>><br>
>> The original patch is split into 6 pieces attached (need applying them<br>
>> in that order):<br>
>><br>
>> * 0001-unify-the-logic-in-SelectAtomicLoadAdd-and-SelectAto.patch<br>
>><br>
>> merge SelectAtomicLoadAdd & SelectAtomicLoadArith into a single<br>
>> function. atomic op table is extended to support ADD/SUB/INC/DEC.<br>
>><br>
>> * 0002-refine-code-generation-for-atomic-operations-with-sp.patch<br>
>><br>
>> revise atomic operations based on spin-loop, reduce one unnecessary load<br>
>> and clean-up/merge the functions. td fils are also cleaned up by<br>
>> eliminating unnecessary attributes for pseudo insn.<br>
>><br>
>> * 0003-revise-atomic-instruction-td-files.patch<br>
>><br>
>> rewrite most of atomic instructions in templates<br>
>><br>
>> * 0004-add-missing-i8-max-min-umax-umin-support.patch<br>
>><br>
>> add missing i8 max/min/umax/umin support<br>
>><br>
>> * 0005-add-missing-i64-max-min-umax-umin-on-32-bit-target.patch<br>
>><br>
>> add missing i64 max/min/umax/umin support on 32-bit target<br>
>><br>
>> * 0006-add-lock-prefix-output-support-in-assembly-printer.patch<br>
>><br>
>> enable assembly printer to output 'lock' prefix to simplify mnemonic in<br>
>> td files<br>
>><br>
>> test cases are added for the previously missing atomic operatons.<br>
>><br>
>> Yours<br>
>> - Michael<br>
>><br>
>><br>
>> ><br>
>> ><br>
>> > On Wed, Jul 25, 2012 at 3:06 PM, Michael Liao <<a href="mailto:michael.liao@intel.com">michael.liao@intel.com</a>><br>
>> > wrote:<br>
>> > - unify the logic in SelectAtomicLoadAdd and<br>
>> > SelectAtomicLoadArith and<br>
>> > merge them together<br>
>> > - refine spin-loop to reduce one unnecessary load<br>
>> > - add missing i8 max/min/umax/umin<br>
>> > - add missing i64 max/min/umax/umin on 32-bit target<br>
>> > - refine atomic instruction td files to use the template for<br>
>> > groups of<br>
>> > instructions<br>
>> > - Output 'lock' prefix in assembler printer to simplify the<br>
>> > assembly<br>
>> > text in td files<br>
>> ><br>
>> > Please review the attached patch and commit if it's OK.<br>
>> ><br>
>> > Yours<br>
>> > - Michael<br>
>> ><br>
>> > ---<br>
>> > [1] <a href="http://software.intel.com/file/36945" target="_blank">http://software.intel.com/file/36945</a><br>
>> > [2]<br>
>> > <a href="http://software.intel.com/en-us/blogs/2012/02/07/transactional-synchronization-in-haswell/" target="_blank">http://software.intel.com/en-us/blogs/2012/02/07/transactional-synchronization-in-haswell/</a><br>
>> ><br>
>> > _______________________________________________<br>
>> > llvm-commits mailing list<br>
>> > <a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br>
>> > <a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
>> ><br>
>> ><br>
>> ><br>
>><br>
>> _______________________________________________<br>
>> llvm-commits mailing list<br>
>> <a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br>
>> <a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
><br>
><br>
> _______________________________________________<br>
> llvm-commits mailing list<br>
> <a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br>
> <a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
</div></div></blockquote></div><br></div>