[Mesa-dev] [android-x86-devel] [RFC 6/7] android: enable x86 asm and sse4 for x86 and x86_64
Zhen Wu
wuzhen at jidemail.com
Sat Apr 30 17:36:02 UTC 2016
2016-04-29 18:22 GMT+08:00 Emil Velikov <emil.l.velikov at gmail.com>:
> On 28 April 2016 at 08:34, Chih-Wei Huang <cwhuang at android-x86.org> wrote:
> > From: WuZhen <wuzhen at jidemail.com>
> >
> > Support multilib compilation use runtime feature selection.
> >
> > NO_REF_TASK
> > tested: local run
> >
> > Change-Id: Iee7961effdecde09cfbdaf09455bfb0912699ae3
> > Signed-off-by: Chih-Wei Huang <cwhuang at linux.org.tw>
> > ---
> > Android.common.mk | 21 ++++++++++++++++++---
> > Android.mk | 5 -----
> > src/mapi/Android.mk | 3 +++
> > src/mesa/Android.gen.mk | 25 ++++++++++++++++++-------
> > src/mesa/Android.libmesa_dricore.mk | 20 +++++++++-----------
> > src/mesa/Android.libmesa_sse41.mk | 5 ++++-
> > src/mesa/Android.libmesa_st_mesa.mk | 20 +++++++++-----------
> > src/mesa/Android.mesa_gen_matypes.mk | 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 libmesa_sse41.mk (effectively adding it to
> st_mesa) - good idea/bugfix.
> Take a look into configure.ac'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.
>
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?
- Adds separate (x86 and x86-64) SSE4.1 flags and libraries - not
> needed afaict, drop ?
>
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.
- 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]
>
No, we have not. We're trying to be in line to other platforms.
> Based on the above this should (ideally) be 3 patches. There are some
> further comment below - please take a look.
>
> > diff --git a/Android.common.mk b/Android.common.mk
> > index d6bf717..93c1465 100644
> > --- a/Android.common.mk
> > +++ b/Android.common.mk
> > @@ -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.
>
> > $(abi_header): PRIVATE_PRINTER := shared-glapi
> >
> > mapi_abi_headers += $(abi_header)
> > diff --git a/src/mesa/Android.gen.mk b/src/mesa/Android.gen.mk
> > index e567102..e04482b 100644
> > --- a/src/mesa/Android.gen.mk
> > +++ b/src/mesa/Android.gen.mk
> > @@ -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) $< > $@
> Have you've tried building this on a non x86 based system ? Does the
> binaries work properly afterwords ?
>
> > diff --git a/src/mesa/Android.libmesa_dricore.mk b/src/mesa/
> Android.libmesa_dricore.mk
> > index d7647a7..6773b5b 100644
> > --- a/src/mesa/Android.libmesa_dricore.mk
> > +++ b/src/mesa/Android.libmesa_dricore.mk
> > @@ -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 \
> > +
> Similar to the defines - one should not need the separate variables
> (libraries) and/or hardcoded behaviour.
>
> > include $(LOCAL_PATH)/Android.gen.mk
> > include $(MESA_COMMON_MK)
> > include $(BUILD_STATIC_LIBRARY)
> > diff --git a/src/mesa/Android.libmesa_sse41.mk b/src/mesa/
> Android.libmesa_sse41.mk
> > index 8562da6..3668785 100644
> > --- a/src/mesa/Android.libmesa_sse41.mk
> > +++ b/src/mesa/Android.libmesa_sse41.mk
> > @@ -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 \
>
> > LOCAL_C_INCLUDES := \
> > $(MESA_TOP)/src/mapi \
> > $(MESA_TOP)/src/gallium/include \
> > diff --git a/src/mesa/Android.libmesa_st_mesa.mk b/src/mesa/
> Android.libmesa_st_mesa.mk
> > index bbd3956..fc97a0a 100644
> > --- a/src/mesa/Android.libmesa_st_mesa.mk
> > +++ b/src/mesa/Android.libmesa_st_mesa.mk
> > @@ -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)/Android.gen.mk
> > include $(MESA_COMMON_MK)
> > diff --git a/src/mesa/Android.mesa_gen_matypes.mk b/src/mesa/
> Android.mesa_gen_matypes.mk
> > index 4fcf73a..163f0e2 100644
> > --- a/src/mesa/Android.mesa_gen_matypes.mk
> > +++ b/src/mesa/Android.mesa_gen_matypes.mk
> > @@ -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
> >
> > --
> > 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 android-x86-devel+unsubscribe at googlegroups.com.
> > To post to this group, send email to android-x86-devel at googlegroups.com.
> > To view this discussion on the web visit
> https://groups.google.com/d/msgid/android-x86-devel/1461828900-4813-7-git-send-email-cwhuang%40linux.org.tw
> .
> > For more options, visit https://groups.google.com/d/optout.
>
> --
> 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 android-x86-devel+unsubscribe at googlegroups.com.
> To post to this group, send email to android-x86-devel at googlegroups.com.
> To view this discussion on the web visit
> https://groups.google.com/d/msgid/android-x86-devel/CACvgo50N29G7vnohOAFZWDWfyNxe2Pu%2BM7LQAprT2Lw8%3D%2BRGhQ%40mail.gmail.com
> .
> For more options, visit https://groups.google.com/d/optout.
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20160501/e65c4bb6/attachment-0001.html>
More information about the mesa-dev
mailing list