[llvm-commits] PATCH: Add LLVM_ENABLE_STAGING option to CMake and configure

Oliveira, Victor Victor.Oliveira at amd.com
Wed Aug 8 17:37:57 PDT 2012


Hi Justin,

Eric wants to say that we don't need a list with all experimental backends to choose from, we should just give the name of the target and that's it.
This way we simplify the development process and just when the backend leaves the staging area we change the build files.
________________________________________
From: llvm-commits-bounces at cs.uiuc.edu [llvm-commits-bounces at cs.uiuc.edu] on behalf of Justin Holewinski [justin.holewinski at gmail.com]
Sent: Wednesday, August 08, 2012 5:17 PM
To: 'Eric Christopher'; 'Chandler Carruth'
Cc: Stellard, Thomas; llvm-commits at cs.uiuc.edu; Villmow, Micah
Subject: Re: [llvm-commits] PATCH: Add LLVM_ENABLE_STAGING option to    CMake   and configure

I'm not sure I understand what you mean.  Adding an experimental target to
the tree just requires it to be listed in a different place.

-----Original Message-----
From: llvm-commits-bounces at cs.uiuc.edu
[mailto:llvm-commits-bounces at cs.uiuc.edu] On Behalf Of Eric Christopher
Sent: Wednesday, August 8, 2012 7:58 PM
To: Chandler Carruth
Cc: Villmow, Micah; Stellard, Thomas; llvm-commits at cs.uiuc.edu LLVM
Subject: Re: [llvm-commits] PATCH: Add LLVM_ENABLE_STAGING option to CMake
and configure

I think my only issue is that you've got it where you want to have to fill
out a table in order to get an experimental target into the tree. If you
want that then OK, but I think you should just auto-add anything in the list
straight to the targets to build.

That may be just me though.

-eric

On Aug 8, 2012, at 4:42 PM, Chandler Carruth <chandlerc at google.com> wrote:

> FWIW, the cmake here LGTM. I'll leave the autoconf etc to others...
>
>
> On Wed, Aug 8, 2012 at 4:39 PM, Oliveira, Victor <Victor.Oliveira at amd.com>
wrote:
> Sure, it makes sense.
>
> Here is another version of the patch that adds a
LLVM_EXPERIMENTAL_TARGETS_TO_BUILD variable to CMake.
> ________________________________________
> From: Justin Holewinski [justin.holewinski at gmail.com]
> Sent: Wednesday, August 08, 2012 1:42 PM
> To: Oliveira, Victor
> Cc: Eric Christopher; Villmow, Micah; David at dcs-maillist.cs.uiuc.edu;
> llvm-commits at cs.uiuc.edu; Stellard, Thomas; Röthlisberger
> Subject: Re: [llvm-commits] PATCH: Add LLVM_ENABLE_STAGING option to
> CMake and configure
>
> So we no longer want an ENABLE_EXPERIMENTAL flag in the CMake system, just
add the target to LLVM_TARGETS_TO_BUILD?  It seems the consensus for auto
tools is to maintain two separate lists and combine them in the end.  Maybe
we should do the same in CMake?  We could have LLVM_TARGETS_TO_BUILD and
LLVM_EXPERIMENTAL_TARGETS_TO_BUILD, and merge them ins to
LLVM_TARGETS_TO_BUILD at configure-time.  I'm all for maintaining
consistency between the auto tools and CMake build systems.
>
> On Aug 7, 2012, at 1:33 PM, "Oliveira, Victor" <Victor.Oliveira at amd.com>
wrote:
>
> > Hi!
> >
> > This patch just adds a --enable-experimental-targets=x,y,z option in
configure. In CMake, you have to explicitly add the backend names in
TARGETS_TO_BUILD (the default option -- all -- just builds non-experimental
backends).
> >
> > What do you think?
> > Victor
> > ________________________________________
> > From: llvm-commits-bounces at cs.uiuc.edu
> > [llvm-commits-bounces at cs.uiuc.edu] on behalf of Eric Christopher
> > [echristo at apple.com]
> > Sent: Friday, August 03, 2012 10:30 AM
> > To: Villmow, Micah
> > Cc: David at dcs-maillist.cs.uiuc.edu; llvm-commits at cs.uiuc.edu;
> > Stellard, Thomas; Röthlisberger
> > Subject: Re: [llvm-commits] PATCH: Add LLVM_ENABLE_STAGING option to
> > CMake and configure
> >
> > The docs seem to be duplicated twice and I might prefer just a simple
--enable-experimental-targets=x,y,z just like --enable-targets=x,y,z and
just add those to TARGETS_TO_BUILD.
> >
> > -eric
> >
> > On Aug 3, 2012, at 10:12 AM, "Villmow, Micah" <Micah.Villmow at amd.com>
wrote:
> >
> >> Ping! Any autotools users that can comment on this patch?
> >>
> >>
> >> Thanks,
> >> Micah
> >>
> >>> -----Original Message-----
> >>> From: llvm-commits-bounces at cs.uiuc.edu [mailto:llvm-commits-
> >>> bounces at cs.uiuc.edu] On Behalf Of Tom Stellard
> >>> Sent: Thursday, August 02, 2012 6:30 AM
> >>> To: Justin Holewinski
> >>> Cc: llvm-commits at cs.uiuc.edu; Röthlisberger; David at dcs-
> >>> maillist.cs.uiuc.edu
> >>> Subject: Re: [llvm-commits] PATCH: Add LLVM_ENABLE_STAGING option
> >>> to CMake and configure
> >>>
> >>> On Tue, Jul 31, 2012 at 12:24:33PM -0400, Justin Holewinski wrote:
> >>>>
> >>>> On 07/31/2012 10:45 AM, Tom Stellard wrote:
> >>>>> On Mon, Jul 30, 2012 at 06:32:53PM -0400, Justin Holewinski wrote:
> >>>>>> On Mon, Jul 30, 2012 at 4:10 PM, Tom Stellard
> >>> <thomas.stellard at amd.com>wrote:
> >>>>>>
> >>>>>>> On Mon, Jul 30, 2012 at 03:01:40PM -0400, Justin Holewinski wrote:
> >>>>>>>> The only thing I would add (other than the autotools
> >>>>>>>> version), is
> >>> a
> >>>>>>>> more descriptive error message for when a staged back-end is
> >>> added
> >>>>>>>> to LLVM_TARGETS_TO_BUILD without setting LLVM_ENABLE_STAGING.
> >>>>>>>>
> >>>>>>> Here is version 2 of the cmake patch with a more descriptive
> >>>>>>> error message and also a new patch that adds the
> >>>>>>> --enable-staging-
> >>> targets option
> >>>>>>> to the configure script.
> >>>>>>>
> >>>>>> Looks good to me, with the caveat that my autoconf-fu is pretty
> >>> weak.
> >>>>>>
> >>>>>>
> >>>>> Here are updated patches with s/STAGING/EXPERIMENTAL/.
> >>>>>
> >>>>> -Tom
> >>>>
> >>>> Looks good to me.
> >>>>
> >>>
> >>> Thanks.
> >>>
> >>> Are there any autotool users that care to comment on the configure
> >>> patch?
> >>>
> >>> -Tom
> >>>
> >>>>>
> >>>>>
> >>>>>>> -Tom
> >>>>>>>
> >>>>>>>
> >>>>>>>> On 07/30/2012 02:10 PM, Tom Stellard wrote:
> >>>>>>>>> Hi,
> >>>>>>>>>
> >>>>>>>>> The attached patched adds the LLVM_ENABLE_STAGING flag to CMake.
> >>> This
> >>>>>>>>> flag is disabled by default and must be enabled to build
> >>>>>>>>> staging targets.  There are no staging targets at the
> >>>>>>>>> moment, so the
> >>> flag
> >>>>>>>>> doesn't actually do anything yet.
> >>>>>>>>>
> >>>>>>>>> The idea for this patch comes from the LLVM staging area
> >>> discussion
> >>>>>>> thread[1]
> >>>>>>>>> on LLVM dev.
> >>>>>>>>>
> >>>>>>>>> Please Review.
> >>>>>>>>>
> >>>>>>>>> Thanks,
> >>>>>>>>> Tom
> >>>>>>>>>
> >>>>>>>>> [1] http://lists.cs.uiuc.edu/pipermail/llvmdev/2012-
> >>> July/052146.html
> >>>>>>>>>
> >>>>>>>>>
> >>>>>>>>> _______________________________________________
> >>>>>>>>> llvm-commits mailing list
> >>>>>>>>> llvm-commits at cs.uiuc.edu
> >>>>>>>>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
> >>>>>>>> --
> >>>>>>>> Thanks,
> >>>>>>>>
> >>>>>>>> Justin Holewinski
> >>>>>>>>
> >>>>>>>> _______________________________________________
> >>>>>>>> llvm-commits mailing list
> >>>>>>>> llvm-commits at cs.uiuc.edu
> >>>>>>>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
> >>>>>>>
> >>>>>>
> >>>>>> --
> >>>>>>
> >>>>>> Thanks,
> >>>>>>
> >>>>>> Justin Holewinski
> >>>>
> >>>> --
> >>>> Thanks,
> >>>>
> >>>> Justin Holewinski
> >>>>
> >>>>
> >>>
> >>> _______________________________________________
> >>> llvm-commits mailing list
> >>> llvm-commits at cs.uiuc.edu
> >>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
> >>
> >> <add-staging-autoconf-option-v2.patch><add-staging-cmake-option-v3.
> >> patch>_______________________________________________
> >> 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
> >
> > <experimental_targets.patch>________________________________________
> > _______
> > 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


_______________________________________________
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






More information about the llvm-commits mailing list