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

Chandler Carruth chandlerc at google.com
Mon Jul 30 16:32:39 PDT 2012


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.

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.


On Mon, Jul 30, 2012 at 4:29 PM, Sean Silva <silvas at purdue.edu> wrote:

> [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
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20120730/9f877218/attachment.html>


More information about the llvm-commits mailing list