bugThe FreeType Project - Bugs: bug #57026, Detect emscripten support for...

 
 

You are not allowed to post comments on this tracker with your current authentication level.

bug #57026: Detect emscripten support for vector attribute.

Submitter:  None
Submitted:  Wed 09 Oct 2019 02:46:26 PM UTC
   
 
Severity:  3 - Normal Item Group:  None
Status:  None Privacy:  Public
Assigned to:  podtelez Open/Closed:  Open
Planned Release:  None

Jump to the original submission

Fri 11 Oct 2019 08:58:30 PM UTC, comment #8: 



Fair enough. gcc did not buy into __has_builtin, not did they document NEON or give an example of proper macro condition. Am I supposed to run these experiments to find out (btw, thank you)?

Perhaps, this is why clang got tired and dumped __builtin_shuffle.

Is it __ARM_NEON__ or __ARM_NEON?

Really, how am I supposed to use such poorly documented non-portable feature?




Alexei Podtelezhnikov <podtelez>
Group Member
Fri 11 Oct 2019 07:07:39 PM UTC, comment #7: 

I've attached the disassembly of premultiply_data before and after the addition of __SSE__ for an aarch64 target. Note that all the vector instructions are gone (as expected) since aarch64 uses __ARM_NEON and does not use __SSE__ (as would be expected, they are different instruction sets). In other words, __SSE__ implies some form of SIMD, but just because SIMD is available (or vector extensions or __builtin_suffle are available) does not imply __SSE__.

In other words, the change is a regression for at least aarch64.

bungeman <bungeman>
Fri 11 Oct 2019 04:26:07 PM UTC, comment #6: 

__builtin_suffle is not portable even with pure GCC. Instead...


On some targets, the instruction set contains SIMD vector instructions which operate on multiple values contained in one large register at the same time. For example, on the x86 the MMX, 3DNow! and SSE extensions can be used this way.


Therefore, it is correct to check for __SSE__ rather than hope for the best.

Of course you can use autoconf to detect it and pass -msse that defines __SSE__ explicitly. This is definitely not the case to be crafty in the source files.


Alexei Podtelezhnikov <podtelez>
Group Member
Fri 11 Oct 2019 03:24:05 PM UTC, comment #5: 

I'm not a big fan of just _SSE_ since the vector extensions were created so they could be at least a bit portable. For example, at the very least _ARM_NEON should also be tested for. Note the emcc compiler doesn't always set __SSE_ when it can do SIMD, it may set the _wasm_simdxxx_ as in the original patch instead (since it is, in fact, a different kind of SIMD).

This is why I proposed an emscripten specific patch, because emcc unfortunately does understand these vector extensions in the front end (the initial pass of the compiler is fine) as the clang it is based on does support them (it's just when the backend target doesn't support the vector extensions that things currently go wonky). It is similar to the icc issue where it claims to support more gcc than it really does, requiring icc detection to 'remove' the support that the reported gcc version would imply.

bungeman <bungeman>
Fri 11 Oct 2019 02:27:14 AM UTC, comment #4: 

Applied this. It will probably still fail for Intel compiler. So help with _mm_shuffle_epi32 needed.

Alexei Podtelezhnikov <podtelez>
Group Member
Thu 10 Oct 2019 01:48:17 PM UTC, comment #3: 

gcc predefines __SSE__ on appropriate architectures even without -msse. I hope clang does too. We should be fine with a simple check tonight. The autoconf script changes are optional, they may or may not come later.

Alexei Podtelezhnikov <podtelez>
Group Member
Thu 10 Oct 2019 11:16:15 AM UTC, comment #2: 

Alexei, please take care of this.  Maybe it makes sense to apply Ben's patch as-is, trying to improve it later on?

Werner LEMBERG <wl>
Group administrator
Wed 09 Oct 2019 09:31:28 PM UTC, comment #1: 

There was another complaint about it.

I would prefer FT_CONFIG_OPTION_NO_VECTOR_EXTENSIONS or indeed _SSE_ without preconditions. _SSE_ would require -msse for autoconf to add.

Alexei Podtelezhnikov <podtelez>
Group Member
Wed 09 Oct 2019 02:46:26 PM UTC, original submission:  

It appears that pngshim.c is the one place FreeType makes (conditional) use of gcc/clang vector extensions. Emscripten has an odd relationship with these vector extensions and only sometimes supports them depending on flags passed by the user. I believe the attached patch does detects when emscripten supports these extensions. A possible alternative would be to have a FT_CONFIG_OPTION_NO_VECTOR_EXTENSIONS config option, much like FT_CONFIG_OPTION_NO_ASSEMBLER.

As a side note, it appears emscripten currently pushes FreeType 2.4.4 [0] as its FreeType 'port'. As far as I can tell this change allows use of tip of tree FreeType with emscripten without further modification (beyond selecting the right options in ftoption.h).


[0] https://github.com/emscripten-core/emscripten/tree/master/tests/freetype

Anonymous

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attached Files

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by bungeman (Posted a comment)
  • -email is unavailable- added by podtelez
  • -email is unavailable- added by podtelez
  • -email is unavailable- added by wl (Posted a comment)
  • -email is unavailable- added by podtelez (Posted a comment)
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

     

    Follow 4 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2019-10-10 podtelez Carbon-Copy- Added bungeman
        Carbon-Copy- Added behdad
    2019-10-10 wl Assigned toNone podtelez
    2019-10-09 None Attached File- Added 0001-Add-emscripten-to-vector-detection.patch, #47633

    Back to the top

    Powered by Savane 3.13-cf05.
    Corresponding source code