Re: [PATCH] build: disable sparse-llvm on non-x86

[Luc Van Oostenryck <luc.vanoostenryck@gmail.com>]

On Tue, Sep 12, 2017 at 9:01 AM, Christopher Li <sparse@chrisli.org> wrote:
> On Sat, Sep 9, 2017 at 9:56 PM, Luc Van Oostenryck
> <luc.vanoostenryck@gmail.com> wrote:
>>
>>  ifeq ($(HAVE_LLVM),yes)
>> +ifeq ($(shell uname -m | grep -q '\(i386\|x86\)' && echo ok),ok)
>>  LLVM_VERSION:=$(shell $(LLVM_CONFIG) --version)
>>  ifeq ($(shell expr "$(LLVM_VERSION)" : '[3-9]\.'),2)
>>  LLVM_PROGS := sparse-llvm
>> @@ -106,6 +107,9 @@ else
>>  $(warning LLVM 3.0 or later required. Your system has version $(LLVM_VERSION) installed.)
>>  endif
>>  else
>> +$(warning sparse-llvm disabled on $(shell uname -m))
>> +endif
>> +else
>>  $(warning Your system does not have llvm, disabling sparse-llvm)
>>  endif
>>
>
> BTW, while I am looking at this, I think the if else testing is getting
> a bit too deep for the rules define of sparse-llvm.
> Right now we have three excuses not to compile llvm:
> 1) not x86,
> 2) LLVM version too old
> 3) Host does not have llvm.
> All of those testing mixing with the actual llvm rules and flags.
>
> I think we can test three level of excuses first, then come to
> conclusion of ENABLE_LLVM(or CONFIG_LLVM) or not.
>
> The rules that define sparse-llvm related stuff should just
> put inside one level of ENABLE_LLVM.
> some thing like:
>
> ifeq ($(ENABLE_LLVM),yes)
>     LLVM_LDFLAGS = ...
>     other llvm flags and rules.
> endif
>
> Do you want to come up with V2? Or I can apply your current patch
> first then do the incremental update on master to use ENABLE_LLVM
> or CONFIG_LLVM
>
> Which way do you prefer?

Please do as is the easiest for you.
I don't mind as I don't have something that depend on it.

-- Luc