[llvm-commits] [PATCH] revise/enhance atomic primitive code generation

[Sean Silva]

[Chandler]
> Can you split this into separate patches and post them individually for review?

The way I understand "post them individually for review" is that I
think he meant to have a separate email for each one, so that each
patch can get a separate thread for discussion. This makes the review
process a lot easier and more focused.

I wouldn't send 6 separate emails all at once though. Just send the
first patch in an email. Once that one has been reviewed and
committed, send the second one, etc. It's possible that the review for
the one patch will give you good ideas for improving the other ones,
so it is best if the reviewers only have to point things out once.
Also, since the patches depend on each other, review for one patch may
force the other patches to change.

--Sean Silva

On Mon, Jul 30, 2012 at 4:02 PM, Michael Liao <michael.liao at intel.com> wrote:
> Hi,
>
> Not sure have any one got chance to review this?
>
> Yours
> - Michael
>
> On Thu, 2012-07-26 at 02:26 -0700, Michael Liao wrote:
>> On Wed, 2012-07-25 at 15:34 -0700, Chandler Carruth wrote:
>> > First off, these sound like fantastic enhancements! =] Thanks for
>> > pulling them out.
>> >
>> >
>> > Can you split this into separate patches and post them individually
>> > for review? I think each of your bullet points would make for a good
>> > patch to review.
>>
>> The original patch is split into 6 pieces attached (need applying them
>> in that order):
>>
>> * 0001-unify-the-logic-in-SelectAtomicLoadAdd-and-SelectAto.patch
>>
>> merge SelectAtomicLoadAdd & SelectAtomicLoadArith into a single
>> function. atomic op table is extended to support ADD/SUB/INC/DEC.
>>
>> * 0002-refine-code-generation-for-atomic-operations-with-sp.patch
>>
>> revise atomic operations based on spin-loop, reduce one unnecessary load
>> and clean-up/merge the functions. td fils are also cleaned up by
>> eliminating unnecessary attributes for pseudo insn.
>>
>> * 0003-revise-atomic-instruction-td-files.patch
>>
>> rewrite most of atomic instructions in templates
>>
>> * 0004-add-missing-i8-max-min-umax-umin-support.patch
>>
>> add missing i8 max/min/umax/umin support
>>
>> * 0005-add-missing-i64-max-min-umax-umin-on-32-bit-target.patch
>>
>> add missing i64 max/min/umax/umin support on 32-bit target
>>
>> * 0006-add-lock-prefix-output-support-in-assembly-printer.patch
>>
>> enable assembly printer to output 'lock' prefix to simplify mnemonic in
>> td files
>>
>> test cases are added for the previously missing atomic operatons.
>>
>> Yours
>> - Michael
>>
>>
>> >
>> >
>> > On Wed, Jul 25, 2012 at 3:06 PM, Michael Liao <michael.liao at intel.com>
>> > wrote:
>> >         - unify the logic in SelectAtomicLoadAdd and
>> >         SelectAtomicLoadArith and
>> >           merge them together
>> >         - refine spin-loop to reduce one unnecessary load
>> >         - add missing i8 max/min/umax/umin
>> >         - add missing i64 max/min/umax/umin on 32-bit target
>> >         - refine atomic instruction td files to use the template for
>> >         groups of
>> >           instructions
>> >         - Output 'lock' prefix in assembler printer to simplify the
>> >         assembly
>> >           text in td files
>> >
>> >         Please review the attached patch and commit if it's OK.
>> >
>> >         Yours
>> >         - Michael
>> >
>> >         ---
>> >         [1] http://software.intel.com/file/36945
>> >         [2]
>> >         http://software.intel.com/en-us/blogs/2012/02/07/transactional-synchronization-in-haswell/
>> >
>> >         _______________________________________________
>> >         llvm-commits mailing list
>> >         llvm-commits at cs.uiuc.edu
>> >         http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>> >
>> >
>> >
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits