Build errors with gcc 14

[jkhsjdhjs]

Hey,

with gcc (GCC) 14.1.1 20240522 I see the following errors when attempting to build p4c:

In function ‘CountTrailingZeroesNonzero16’,
    inlined from ‘CountTrailingZeroes’ at /usr/include/absl/numeric/internal/bits.h:324:24,
    inlined from ‘countr_zero’ at /usr/include/absl/numeric/bits.h:119:47,
    inlined from ‘TrailingZeros’ at /usr/include/absl/container/internal/raw_hash_set.h:374:43,
    inlined from ‘LowestBitSet’ at /usr/include/absl/container/internal/raw_hash_set.h:395:45,
    inlined from ‘operator*’ at /usr/include/absl/container/internal/raw_hash_set.h:452:57,
    inlined from ‘find’ at /usr/include/absl/container/internal/raw_hash_set.h:2694:7,
    inlined from ‘find’ at /usr/include/absl/container/internal/raw_hash_set.h:2714:49,
    inlined from ‘find.isra’ at /usr/include/absl/container/internal/raw_hash_set.h:2720:16:
/usr/include/absl/numeric/internal/bits.h:309:24: error: ‘__builtin_ctzs’ needs isa option -mbmi
  309 |   return __builtin_ctzs(x);
      |

(and more with the same error...)

As suggested by the error, adding

add_compile_options(-mbmi)

to the main CMakeLists.txt fixes the issue, but I don't think that's a proper fix and I don't know what a proper fix would look like.

[ChrisDodd]

This is an unfortunate regression introduced by gcc. ctzs can be implemented without the bmi instructions, but it is slightly less efficient (require 2 instructions instead of 1). Not clear why gcc decided to remove that implementation.

[ChrisDodd]

An alternate (better?) fix would be to compile abseil with -D__builtin_ctzs=__builtin_ctz or have a patch we apply to it to change the ctzs into a ctz

[asl]

This issue was fixed in abseil upstream: abseil/abseil-cpp#1664 Also, it seems we also had workaround at #4638

[ChrisDodd]

The abseil fix seems best -- though it is really a gcc bug (IMO). Not sure which versions of gcc support the -g versions of builtins, but as long as all the versions we care about do, it should be ok.

[asl]

The abseil fix seems best -- though it is really a gcc bug (IMO). Not sure which versions of gcc support the -g versions of builtins, but as long as all the versions we care about do, it should be ok.

Yes. This is a mainline GCC bug. Fortunately, we saw it early as Fedora started to use unreleased version of gcc :) Abseil fix landed ~week ago, so it will be in the next LTS release (I hope). IIRC -g variants are supported by gcc 14 (released) and clang 19 (added llvm/llvm-project@c1c2551, will be released end July).

[ChrisDodd]

IIRC -g variants are supported by gcc 14 and clang 19 (llvm/llvm-project@c1c2551).

Do older versions support them? I think we still care about gcc-11 at least, and perhaps gcc-9 as well. It looks like the way the abseil patch works is to use the -g variant if that is available and the -s version otherwise, and on gcc prior to 14 that works by (gcc internally) falling back to the basic int version.

[asl]

It looks like the way the abseil patch works is to use the -g variant if that is available and the -s version otherwise, and on gcc prior to 14 that works by (gcc internally) falling back to the basic int version.

Right. They fallback to other implementations, if they are available. So, essentially:

• Issue was introduced in GCC 14. Fine, -g variant will be used with gcc 14+, so no issue with the fix.
• Before gcc 14 other variants (e.g. -s or; or, if not available, fully generic implementation) will be used. Use of -s variant does not have BMI requirement.

[jkhsjdhjs]

Also, it seems we also had workaround at #4638

Interesting, the fix is contained in the latest release, I wonder why it doesn't work for me.

[jkhsjdhjs]

Ah I see, the workaround is only for the P4C_USE_PREINSTALLED_ABSEIL=OFF branch. Guess I'll just wait for the next abseil or gcc update.