[Mesa-dev] [PATCH v3 02/25] configure.ac: Add helper function for targets/components

[Tobias Droste]

Am Mittwoch, 12. Oktober 2016, 19:38:56 CEST schrieb Emil Velikov:
> On 12 October 2016 at 19:01, Tobias Droste <tdroste at gmx.de> wrote:
> > Am Mittwoch, 12. Oktober 2016, 09:52:51 CEST schrieb Emil Velikov:
> >> On 12 October 2016 at 00:02, Tobias Droste <tdroste at gmx.de> wrote:
> >> > Add functions to add and check targets/components.
> >> > Not used in this patch.
> >> > 
> >> > The error message in llvm_add_component is disabled until it doesn't
> >> > break
> >> > the build anymore. This is the same functionality as before where the
> >> > components were added without a check.
> >> > 
> >> > Signed-off-by: Tobias Droste <tdroste at gmx.de>
> >> > ---
> >> > 
> >> >  configure.ac | 35 +++++++++++++++++++++++++++++++++++
> >> >  1 file changed, 35 insertions(+)
> >> > 
> >> > diff --git a/configure.ac b/configure.ac
> >> > index bdd46bc..69421ff 100644
> >> > --- a/configure.ac
> >> > +++ b/configure.ac
> >> > @@ -2196,7 +2196,42 @@ llvm_check_version_for() {
> >> > 
> >> >      fi
> >> >  
> >> >  }
> >> > 
> >> > +llvm_add_default_components() {
> >> > +    driver_name=$1
> >> > 
> >> > +    # Required default components
> >> > +    llvm_add_component "engine" $driver_name
> >> > +    llvm_add_component "bitwriter" $driver_name
> >> > +    llvm_add_component "mcjit" $driver_name
> >> > +    llvm_add_component "mcdisassembler" $driver_name
> >> > +
> >> > +    # Optional default components
> >> > +    if $LLVM_CONFIG --components | grep -iqw inteljitevents ; then
> >> > +        LLVM_COMPONENTS="${LLVM_COMPONENTS} inteljitevents"
> >> > +    fi
> >> > +}
> >> > +
> >> > +llvm_add_component() {
> >> 
> >> Nit: move this function before its user (llvm_add_default_components)
> >> Idea: call this ...components (plural) and feed the all components at
> >> once.
> > 
> > Ok.
> 
> Just realised that you're adding these only to move them further up
> with later patch. What's the obstacle for doing so in the first place
> ?

Nothing, but I was already on path 20 when I realised it and was to lazy to 
switch the ordering of the patches ;-)

> 
> >> > +    new_llvm_component=$1
> >> > +    driver_name=$2
> >> > +
> >> > +    if $LLVM_CONFIG --components | grep -iqw $new_llvm_component ;
> >> > then
> >> > +        LLVM_COMPONENTS="${LLVM_COMPONENTS} ${new_llvm_component}"
> >> > +    #else
> >> > +    #    AC_MSG_ERROR([LLVM component '$new_llvm_component' not
> >> > enabled
> >> > in your LLVM build. Required by $driver_name.])
> >> 
> >> This function adds required components, correct ? Then the above two
> >> lines should not be commented out.
> > 
> > I can't enable this until later in the series as mentioned in the commit
> > message.
> > This is how it worked before (there were no errors for the components).
> > 
> > At this point llvm_add_default_components will be added even if we don't
> > need LLVM. This way someone without LLVM installed will get error
> > messages even though he might just want to build the intel or softpipe
> > driver.
> > It will be enabled in Patch 24.
> 
> If things error out here, that means that one should finish the other
> refactoring before using these helpers ?
> If that's getting too hairy, just add keep the function a simple
> wrapper around LLVM_COMPONENTS (no LLVM_CONFIG or AC_MSG_ERROR) and
> add those when it's safe.

Right now this is just a wrapper around LLVM_COMPONENTS.
If a component does not exist it will later not generate any LLVM_LIBS that's 
why it also doesn't need to be in LLVM_COMPONENTS.
If it actually is missing, you will only notice during linking / building 
(depending on wether headers are missing or not).

> 
> -Emil