<div dir="ltr"><div>2016-04-29 18:22 GMT+08:00 Emil Velikov <<a href="mailto:emil.l.velikov@gmail.com" target="_blank">emil.l.velikov@gmail.com</a>>: </div><div class="gmail_extra"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex">On 28 April 2016 at 08:34, Chih-Wei Huang <<a href="mailto:cwhuang@android-x86.org">cwhuang@android-x86.org</a>> wrote: > From: WuZhen <<a href="mailto:wuzhen@jidemail.com">wuzhen@jidemail.com</a>> > > Support multilib compilation use runtime feature selection. > > NO_REF_TASK > tested: local run > > Change-Id: Iee7961effdecde09cfbdaf09455bfb0912699ae3 > Signed-off-by: Chih-Wei Huang <<a href="mailto:cwhuang@linux.org.tw">cwhuang@linux.org.tw</a>> > --- > <a href="http://Android.common.mk" rel="noreferrer" target="_blank">Android.common.mk</a> | 21 ++++++++++++++++++--- > Android.mk | 5 ----- > src/mapi/Android.mk | 3 +++ > src/mesa/<a href="http://Android.gen.mk" rel="noreferrer" target="_blank">Android.gen.mk</a> | 25 ++++++++++++++++++------- > src/mesa/<a href="http://Android.libmesa_dricore.mk" rel="noreferrer" target="_blank">Android.libmesa_dricore.mk</a> | 20 +++++++++----------- > src/mesa/<a href="http://Android.libmesa_sse41.mk" rel="noreferrer" target="_blank">Android.libmesa_sse41.mk</a> | 5 ++++- > src/mesa/<a href="http://Android.libmesa_st_mesa.mk" rel="noreferrer" target="_blank">Android.libmesa_st_mesa.mk</a> | 20 +++++++++----------- > src/mesa/<a href="http://Android.mesa_gen_matypes.mk" rel="noreferrer" target="_blank">Android.mesa_gen_matypes.mk</a> | 7 +++++-- > src/mesa/drivers/dri/i965/Android.mk | 5 ----- > 9 files changed, 66 insertions(+), 45 deletions(-) > Afact this patch does a few things: - Moves -DUSE_SSE41 to the top level - good idea [patch 1] - Consolidates -msse41 to <a href="http://libmesa_sse41.mk" rel="noreferrer" target="_blank">libmesa_sse41.mk</a> (effectively adding it to st_mesa) - good idea/bugfix. Take a look into <a href="http://configure.ac" rel="noreferrer" target="_blank">configure.ac</a>'s add -mstackrealign (and the commit that introduced it). You might want the same/similar fix. [patch 2] - Forces SSE4.1 even if there is no support for it. I.e. ARCH_X86_HAVE_SSE4_1 checks were dropped - very bad idea, please don't. </blockquote><div> </div><div>mesa seems always check for SSE4.1 support before using any of sse4.1 functions, if you are talking about compiler support for sse4.1, the toolchain used in android should always support this. I only changed that into always generate sse4.1 and rely on those runtion checks to avoid issues on system without sse4.1 support, do you think this is a issue? </div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex"> - Adds separate (x86 and x86-64) SSE4.1 flags and libraries - not needed afaict, drop ? </blockquote><div> </div><div>I'm trying to avoid not breaking non-x86 builds. this is to seems better than to surround them in a if clause. especially if we're to add flags for other architectures. </div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex"> - Has some trailing '\' in which should cause build warning/errors - see comments below - Gets "asm" building when host_arch != target_arch - have you guys tested it. what is the perf. improvement ? [patch 3] </blockquote><div> </div><div>No, we have not. We're trying to be in line to other platforms.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex"> Based on the above this should (ideally) be 3 patches. There are some further comment below - please take a look. > diff --git a/<a href="http://Android.common.mk" rel="noreferrer" target="_blank">Android.common.mk</a> b/<a href="http://Android.common.mk" rel="noreferrer" target="_blank">Android.common.mk</a> > index d6bf717..93c1465 100644 > --- a/<a href="http://Android.common.mk" rel="noreferrer" target="_blank">Android.common.mk</a> > +++ b/<a href="http://Android.common.mk" rel="noreferrer" target="_blank">Android.common.mk</a> > @@ -64,13 +64,28 @@ LOCAL_CFLAGS += \ > LOCAL_CONLYFLAGS += \ > -std=c99 > > +x86_flags := \ > + -DUSE_SSE41 \ > + > +x86_64_flags := \ > + -DUSE_SSE41 \ > + No need for separate x86/x86_64 variables here. If one does not force the define (as currently done) that is. > ifeq ($(strip $(MESA_ENABLE_ASM)),true) > -ifeq ($(TARGET_ARCH),x86) > -LOCAL_CFLAGS += \ > +x86_flags += \ > -DUSE_X86_ASM \ > + -DUSE_MMX_ASM \ > + -DUSE_3DNOW_ASM \ > + -DUSE_SSE_ASM \ > + > +x86_64_flags += \ > + -DUSE_X86_64_ASM \ > These are unterminated (i.e. trailing \). > endif > -endif > + > +LOCAL_ASFLAGS_x86 += $(x86_flags) > +LOCAL_ASFLAGS_x86_64 += $(x86_64_flags) > +LOCAL_CFLAGS_x86 += $(x86_flags) > +LOCAL_CFLAGS_x86_64 += $(x86_64_flags) > > ifeq ($(MESA_ENABLE_LLVM),true) > LOCAL_CFLAGS += \ > diff --git a/Android.mk b/Android.mk > index 67d894f..368d02e 100644 > --- a/Android.mk > +++ b/Android.mk > @@ -63,12 +63,7 @@ $(warning invalid GPU drivers: $(invalid_drivers)) > MESA_GPU_DRIVERS := $(filter-out $(invalid_drivers), $(MESA_GPU_DRIVERS)) > endif > > -# host and target must be the same arch to generate matypes.h > -ifeq ($(TARGET_ARCH),$(HOST_ARCH)) > MESA_ENABLE_ASM := true Since now it's always enabled one can drop the variable. > -else > -MESA_ENABLE_ASM := false > -endif > > ifneq ($(filter $(classic_drivers), $(MESA_GPU_DRIVERS)),) > MESA_BUILD_CLASSIC := true > diff --git a/src/mapi/Android.mk b/src/mapi/Android.mk > index 4445218..b654c25 100644 > --- a/src/mapi/Android.mk > +++ b/src/mapi/Android.mk > @@ -57,6 +57,9 @@ intermediates := $(call local-generated-sources-dir) > abi_header := $(intermediates)/$(abi_header) > LOCAL_GENERATED_SOURCES := $(abi_header) > > +# workaround build warning > +LOCAL_LDFLAGS_x86 += -Wl,--no-warn-shared-textrel > + Can you elaborate a bit more here. It might be a genuine bug that we want to fix. This is currently hidden amongst many other things which makes it very easy to miss. If one is to include it with another patch it should be mentioned in the commit summary. <div><div class="h5"> > $(abi_header): PRIVATE_PRINTER := shared-glapi > > mapi_abi_headers += $(abi_header) > diff --git a/src/mesa/<a href="http://Android.gen.mk" rel="noreferrer" target="_blank">Android.gen.mk</a> b/src/mesa/<a href="http://Android.gen.mk" rel="noreferrer" target="_blank">Android.gen.mk</a> > index e567102..e04482b 100644 > --- a/src/mesa/<a href="http://Android.gen.mk" rel="noreferrer" target="_blank">Android.gen.mk</a> > +++ b/src/mesa/<a href="http://Android.gen.mk" rel="noreferrer" target="_blank">Android.gen.mk</a> > @@ -45,10 +45,11 @@ LOCAL_SRC_FILES := $(filter-out $(sources), $(LOCAL_SRC_FILES)) > LOCAL_C_INCLUDES += $(intermediates)/main > > ifeq ($(strip $(MESA_ENABLE_ASM)),true) > -ifeq ($(TARGET_ARCH),x86) > -sources += x86/matypes.h > -LOCAL_C_INCLUDES += $(intermediates)/x86 > -endif > +LOCAL_GENERATED_SOURCES_x86 += $(addprefix $(intermediates)/, x86/matypes.h) > +LOCAL_GENERATED_SOURCES_x86_64 += $(addprefix $(intermediates)/, x86_64/matypes.h) > + > +LOCAL_C_INCLUDES_x86 += $(intermediates)/x86 > +LOCAL_C_INCLUDES_x86_64 += $(intermediates)/x86_64 > endif > > sources += main/git_sha1.h > @@ -79,12 +80,22 @@ $(intermediates)/main/git_sha1.h: $(wildcard $(MESA_TOP)/.git/HEAD) > > $@; \ > fi > > -matypes_deps := \ > - $(BUILD_OUT_EXECUTABLES)/mesa_gen_matypes$(BUILD_EXECUTABLE_SUFFIX) \ > +matypes_deps32 := \ > + $(BUILD_OUT_EXECUTABLES)/mesa_gen_matypes32$(BUILD_EXECUTABLE_SUFFIX) \ > + $(LOCAL_PATH)/main/mtypes.h \ > + $(LOCAL_PATH)/tnl/t_context.h > + > +matypes_deps64 := \ > + $(BUILD_OUT_EXECUTABLES)/mesa_gen_matypes64$(BUILD_EXECUTABLE_SUFFIX) \ > $(LOCAL_PATH)/main/mtypes.h \ > $(LOCAL_PATH)/tnl/t_context.h > > -$(intermediates)/x86/matypes.h: $(matypes_deps) > +$(intermediates)/x86/matypes.h: $(matypes_deps32) > + @mkdir -p $(dir $@) > + @echo "MATYPES: $(PRIVATE_MODULE) <= $(notdir $@)" > + $(hide) $< > $@ > + > +$(intermediates)/x86_64/matypes.h: $(matypes_deps64) > @mkdir -p $(dir $@) > @echo "MATYPES: $(PRIVATE_MODULE) <= $(notdir $@)" > $(hide) $< > $@ </div></div>Have you've tried building this on a non x86 based system ? Does the binaries work properly afterwords ? <div><div class="h5"> > diff --git a/src/mesa/<a href="http://Android.libmesa_dricore.mk" rel="noreferrer" target="_blank">Android.libmesa_dricore.mk</a> b/src/mesa/<a href="http://Android.libmesa_dricore.mk" rel="noreferrer" target="_blank">Android.libmesa_dricore.mk</a> > index d7647a7..6773b5b 100644 > --- a/src/mesa/<a href="http://Android.libmesa_dricore.mk" rel="noreferrer" target="_blank">Android.libmesa_dricore.mk</a> > +++ b/src/mesa/<a href="http://Android.libmesa_dricore.mk" rel="noreferrer" target="_blank">Android.libmesa_dricore.mk</a> > @@ -31,6 +31,7 @@ LOCAL_PATH := $(call my-dir) > # Import the following variables: > # MESA_FILES > # X86_FILES > +# X86_64_FILES > include $(LOCAL_PATH)/Makefile.sources > > include $(CLEAR_VARS) > @@ -42,19 +43,10 @@ LOCAL_SRC_FILES := \ > $(MESA_FILES) > > ifeq ($(strip $(MESA_ENABLE_ASM)),true) > -ifeq ($(TARGET_ARCH),x86) > - LOCAL_SRC_FILES += $(X86_FILES) > -endif # x86 > + LOCAL_SRC_FILES_x86 += $(X86_FILES) > + LOCAL_SRC_FILES_x86_64 += $(X86_64_FILES) > endif # MESA_ENABLE_ASM > > -ifeq ($(ARCH_X86_HAVE_SSE4_1),true) > -LOCAL_WHOLE_STATIC_LIBRARIES := \ > - libmesa_sse41 > -LOCAL_CFLAGS := \ > - -msse4.1 \ > - -DUSE_SSE41 > -endif > - > LOCAL_C_INCLUDES := \ > $(MESA_TOP)/src/mapi \ > $(MESA_TOP)/src/mesa/main \ > @@ -65,6 +57,12 @@ LOCAL_C_INCLUDES := \ > LOCAL_WHOLE_STATIC_LIBRARIES += \ > libmesa_program > > +LOCAL_WHOLE_STATIC_LIBRARIES_x86 += \ > + libmesa_sse41 \ > + > +LOCAL_WHOLE_STATIC_LIBRARIES_x86_64 += \ > + libmesa_sse41 \ > + </div></div>Similar to the defines - one should not need the separate variables (libraries) and/or hardcoded behaviour. > include $(LOCAL_PATH)/<a href="http://Android.gen.mk" rel="noreferrer" target="_blank">Android.gen.mk</a> > include $(MESA_COMMON_MK) > include $(BUILD_STATIC_LIBRARY) > diff --git a/src/mesa/<a href="http://Android.libmesa_sse41.mk" rel="noreferrer" target="_blank">Android.libmesa_sse41.mk</a> b/src/mesa/<a href="http://Android.libmesa_sse41.mk" rel="noreferrer" target="_blank">Android.libmesa_sse41.mk</a> > index 8562da6..3668785 100644 > --- a/src/mesa/<a href="http://Android.libmesa_sse41.mk" rel="noreferrer" target="_blank">Android.libmesa_sse41.mk</a> > +++ b/src/mesa/<a href="http://Android.libmesa_sse41.mk" rel="noreferrer" target="_blank">Android.libmesa_sse41.mk</a> > @@ -20,7 +20,7 @@ > # FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER > # DEALINGS IN THE SOFTWARE. > > -ifeq ($(ARCH_X86_HAVE_SSE4_1),true) > +ifneq ($(filter x86 x86_64,$(TARGET_ARCH)),) > > LOCAL_PATH := $(call my-dir) > > @@ -33,6 +33,9 @@ LOCAL_MODULE := libmesa_sse41 > LOCAL_SRC_FILES += \ > $(X86_SSE41_FILES) > > +LOCAL_CFLAGS += \ > + -msse4.1 \ > + Trailing \ <div class=""><div class="h5"> > LOCAL_C_INCLUDES := \ > $(MESA_TOP)/src/mapi \ > $(MESA_TOP)/src/gallium/include \ > diff --git a/src/mesa/<a href="http://Android.libmesa_st_mesa.mk" rel="noreferrer" target="_blank">Android.libmesa_st_mesa.mk</a> b/src/mesa/<a href="http://Android.libmesa_st_mesa.mk" rel="noreferrer" target="_blank">Android.libmesa_st_mesa.mk</a> > index bbd3956..fc97a0a 100644 > --- a/src/mesa/<a href="http://Android.libmesa_st_mesa.mk" rel="noreferrer" target="_blank">Android.libmesa_st_mesa.mk</a> > +++ b/src/mesa/<a href="http://Android.libmesa_st_mesa.mk" rel="noreferrer" target="_blank">Android.libmesa_st_mesa.mk</a> > @@ -41,18 +41,10 @@ LOCAL_SRC_FILES := \ > $(MESA_GALLIUM_FILES) > > ifeq ($(strip $(MESA_ENABLE_ASM)),true) > -ifeq ($(TARGET_ARCH),x86) > - LOCAL_SRC_FILES += $(X86_FILES) > -endif # x86 > + LOCAL_SRC_FILES_x86 += $(X86_FILES) > + LOCAL_SRC_FILES_x86_64 += $(X86_64_FILES) > endif # MESA_ENABLE_ASM > > -ifeq ($(ARCH_X86_HAVE_SSE4_1),true) > -LOCAL_WHOLE_STATIC_LIBRARIES := \ > - libmesa_sse41 > -LOCAL_CFLAGS := \ > - -DUSE_SSE41 > -endif > - > LOCAL_C_INCLUDES := \ > $(MESA_TOP)/src/mapi \ > $(MESA_TOP)/src/mesa/main \ > @@ -61,7 +53,13 @@ LOCAL_C_INCLUDES := \ > $(MESA_TOP)/src/gallium/include > > LOCAL_WHOLE_STATIC_LIBRARIES += \ > - libmesa_program > + libmesa_program \ > + > +LOCAL_WHOLE_STATIC_LIBRARIES_x86 += \ > + libmesa_sse41 \ > + > +LOCAL_WHOLE_STATIC_LIBRARIES_x86_64 += \ > + libmesa_sse41 \ > > include $(LOCAL_PATH)/<a href="http://Android.gen.mk" rel="noreferrer" target="_blank">Android.gen.mk</a> > include $(MESA_COMMON_MK) > diff --git a/src/mesa/<a href="http://Android.mesa_gen_matypes.mk" rel="noreferrer" target="_blank">Android.mesa_gen_matypes.mk</a> b/src/mesa/<a href="http://Android.mesa_gen_matypes.mk" rel="noreferrer" target="_blank">Android.mesa_gen_matypes.mk</a> > index 4fcf73a..163f0e2 100644 > --- a/src/mesa/<a href="http://Android.mesa_gen_matypes.mk" rel="noreferrer" target="_blank">Android.mesa_gen_matypes.mk</a> > +++ b/src/mesa/<a href="http://Android.mesa_gen_matypes.mk" rel="noreferrer" target="_blank">Android.mesa_gen_matypes.mk</a> > @@ -25,13 +25,16 @@ > # --------------------------------------------------------------------- > > ifeq ($(strip $(MESA_ENABLE_ASM)),true) > -ifeq ($(TARGET_ARCH),x86) > +ifneq ($(filter x86 x86_64,$(TARGET_ARCH)),) > > LOCAL_PATH := $(call my-dir) > > include $(CLEAR_VARS) > > LOCAL_MODULE := mesa_gen_matypes > +LOCAL_MULTILIB := both > +LOCAL_MODULE_STEM_32 := $(LOCAL_MODULE)32 > +LOCAL_MODULE_STEM_64 := $(LOCAL_MODULE)64 > LOCAL_IS_HOST_MODULE := true > > LOCAL_C_INCLUDES := \ > @@ -43,5 +46,5 @@ LOCAL_SRC_FILES := \ > include $(MESA_COMMON_MK) > include $(BUILD_HOST_EXECUTABLE) > > -endif # x86 > +endif # x86 x86_64 > endif # MESA_ENABLE_ASM > diff --git a/src/mesa/drivers/dri/i965/Android.mk b/src/mesa/drivers/dri/i965/Android.mk > index 056b223..a82b69b 100644 > --- a/src/mesa/drivers/dri/i965/Android.mk > +++ b/src/mesa/drivers/dri/i965/Android.mk > @@ -39,11 +39,6 @@ include $(LOCAL_PATH)/Makefile.sources > LOCAL_CFLAGS := \ > $(MESA_DRI_CFLAGS) > > -ifeq ($(ARCH_X86_HAVE_SSE4_1),true) > -LOCAL_CFLAGS += \ > - -DUSE_SSE41 > -endif > - > LOCAL_C_INCLUDES := \ > $(MESA_DRI_C_INCLUDES) > > -- > 1.9.1 > </div></div>> -- > You received this message because you are subscribed to the Google Groups "Android-x86 development" group. > To unsubscribe from this group and stop receiving emails from it, send an email to <a href="mailto:android-x86-devel%2Bunsubscribe@googlegroups.com">android-x86-devel+unsubscribe@googlegroups.com</a>. > To post to this group, send email to <a href="mailto:android-x86-devel@googlegroups.com">android-x86-devel@googlegroups.com</a>. > To view this discussion on the web visit <a href="https://groups.google.com/d/msgid/android-x86-devel/1461828900-4813-7-git-send-email-cwhuang%40linux.org.tw" rel="noreferrer" target="_blank">https://groups.google.com/d/msgid/android-x86-devel/1461828900-4813-7-git-send-email-cwhuang%40linux.org.tw</a>. > For more options, visit <a href="https://groups.google.com/d/optout" rel="noreferrer" target="_blank">https://groups.google.com/d/optout</a>. -- You received this message because you are subscribed to the Google Groups "Android-x86 development" group. To unsubscribe from this group and stop receiving emails from it, send an email to <a href="mailto:android-x86-devel%2Bunsubscribe@googlegroups.com">android-x86-devel+unsubscribe@googlegroups.com</a>. To post to this group, send email to <a href="mailto:android-x86-devel@googlegroups.com">android-x86-devel@googlegroups.com</a>. To view this discussion on the web visit <a href="https://groups.google.com/d/msgid/android-x86-devel/CACvgo50N29G7vnohOAFZWDWfyNxe2Pu%2BM7LQAprT2Lw8%3D%2BRGhQ%40mail.gmail.com" rel="noreferrer" target="_blank">https://groups.google.com/d/msgid/android-x86-devel/CACvgo50N29G7vnohOAFZWDWfyNxe2Pu%2BM7LQAprT2Lw8%3D%2BRGhQ%40mail.gmail.com</a>. For more options, visit <a href="https://groups.google.com/d/optout" rel="noreferrer" target="_blank">https://groups.google.com/d/optout</a>. </blockquote></div> </div></div>