boostorg / fiber

userland threads
464 stars 108 forks source link

Unbreak fiber for ELF PowerPC building with Clang #313

Closed brad0 closed 11 months ago

brad0 commented 11 months ago

Clang also defines the uppercase __POWERPC__ symbol so the commit 2bd8eb02f81ab8297aa89b8cb29b0f3867b9fafe breaks the build on ELF PowerPC systems. Since we know this is already a PowerPC system with BOOST_ARCH_PPC then check for the vendor __APPLE__ instead.

brad0 commented 11 months ago

cc @barracuda156

barracuda156 commented 11 months ago

@brad0 Thank you, this is something new to hear. Which OS and which clang? Just so that I know.

brad0 commented 11 months ago

@brad0 Thank you, this is something new to hear. Which OS and which clang? Just so that I know.

OpenBSD, but this would affect any ELF OS. e.g. *BSD, Linux if building with Clang.

barracuda156 commented 11 months ago

Good to know. Should be careful then to always check for __APPLE__ && __POWERPC__. Or at least exclude Clang (__POWERPC__ is also defined on BeOS, AFAIR, but this is not relevant to the present case).

brad0 commented 11 months ago

Good to know. Should be careful then to always check for __APPLE__ && __POWERPC__. Or at least exclude Clang (__POWERPC__ is also defined on BeOS, AFAIR, but this is not relevant to the present case).

symbols defined for Clang PowerPC targets..

  // Target identification.
  Builder.defineMacro("__ppc__");
  Builder.defineMacro("__PPC__");
  Builder.defineMacro("_ARCH_PPC");
  Builder.defineMacro("__powerpc__");
  Builder.defineMacro("__POWERPC__");
  if (PointerWidth == 64) {
    Builder.defineMacro("_ARCH_PPC64");
    Builder.defineMacro("__powerpc64__");
    Builder.defineMacro("__PPC64__");
  }
barracuda156 commented 11 months ago

Good to know. Should be careful then to always check for __APPLE__ && __POWERPC__. Or at least exclude Clang (__POWERPC__ is also defined on BeOS, AFAIR, but this is not relevant to the present case).

symbols defined for Clang PowerPC targets..


  // Target identification.

  Builder.defineMacro("__ppc__");

  Builder.defineMacro("__PPC__");

  Builder.defineMacro("_ARCH_PPC");

  Builder.defineMacro("__powerpc__");

  Builder.defineMacro("__POWERPC__");

  if (PointerWidth == 64) {

    Builder.defineMacro("_ARCH_PPC64");

    Builder.defineMacro("__powerpc64__");

    Builder.defineMacro("__PPC64__");

  }

(On a side note, what a mess: why would it define __ppc__ for 32- and 64-bit both?)

For BeOS, it does use __POWERPC__, but I am not sure about its assembler syntax. It may be using unprefixed registers in fact, judging from https://opensource.apple.com/source/gcc3/gcc3-1175/gcc/config/rs6000/beos.h.auto.html If so, your change may fix BeOS too.

brad0 commented 11 months ago

(On a side note, what a mess: why would it define __ppc__ for 32- and 64-bit both?)

Same goes for __powerpc__ for both.

For BeOS, it does use __POWERPC__, but I am not sure about its assembler syntax. It may be using unprefixed registers in fact, judging from https://opensource.apple.com/source/gcc3/gcc3-1175/gcc/config/rs6000/beos.h.auto.html If so, your change may fix BeOS too.

Even if it had to go the other way you could check add a check for BeOS.

olk commented 11 months ago

ty