Skip to content

Commit 0e629da

Browse files
committed
indexcodec: Refine the cmov codegen workaround for gcc
gcc has the same problem as clang, but it can't be solved using an attribute: gcc doesn't support __builtin_unpredictable and __builtin_expect_with_probability does not affect the codegen here. Additionally, some versions of gcc (gcc-16+, possibly earlier versions) can fuse two reads from edgefifo[] for a/b into a single 64-bit load, but keep separate stores into edgefifo[]. This breaks store to load forwarding, as it usually requires that a load is covered by a single store, which results in further significant performance loss. Both of these can be fixed with an asm barrier: cf is forced to be materialized in a register which prevents folding it into the ternary, at which point the select is simple and emits cmov; the +r restriction on a also blocks fusing the loads into a single load. While clang could technically keep using the attribute workaround, the same workaround appears to work fine for it, so for simplicity we use the same asm barrier for both compilers for now. This makes index decoding ~10-12% faster for gcc14/15 and ~19% faster for gcc16.
1 parent a550854 commit 0e629da

1 file changed

Lines changed: 7 additions & 10 deletions

File tree

src/indexcodec.cpp

Lines changed: 7 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -7,11 +7,6 @@
77
// This work is based on:
88
// Fabian Giesen. Simple lossless index buffer compression & follow-up. 2013
99
// Conor Stokes. Vertex Cache Optimised Index Buffer Compression. 2014
10-
11-
#ifndef __has_builtin
12-
#define __has_builtin(x) 0
13-
#endif
14-
1510
namespace meshopt
1611
{
1712

@@ -450,13 +445,15 @@ int meshopt_decodeIndexBuffer(void* destination, size_t index_count, size_t inde
450445
// fifo reads are wrapped around 16 entry buffer
451446
unsigned int cf = vertexfifo[(vertexfifooffset - 1 - fec) & 15];
452447

453-
// clang needs the branch annotation because cf[] comes from "memory" and x86-cmov-conversion pass removes cmov otherwise
454-
#if defined(__clang__) && __has_builtin(__builtin_unpredictable)
455-
c = __builtin_unpredictable(fec == 0) ? next : cf;
456-
#else
457-
c = (fec == 0) ? next : cf;
448+
#if (defined(__GNUC__) || defined(__clang__)) && (defined(__x86_64__) || defined(__aarch64__))
449+
// on clang, x86-cmov-conversion pass emits a branch since cf is a load from memory; asm barrier defeats this
450+
// this can be fixed with __builtin_unpredictable, but gcc doesn't support it and has a similar problem due to if-conversion
451+
// additionally, gcc can fuse a/b into one 64-bit load but use 32-bit edgefifo[] stores, which breaks load store forwarding
452+
__asm__("" : "+r"(a) : "r"(cf));
458453
#endif
459454

455+
c = (fec == 0) ? next : cf;
456+
460457
int fec0 = fec == 0;
461458
next += fec0;
462459

0 commit comments

Comments
 (0)