bitwiseworks / gcc-os2

Port of GCC compiler to OS/2
GNU General Public License v2.0
16 stars 2 forks source link

error: unsupported size for integer register #3

Closed dmik closed 4 years ago

dmik commented 4 years ago

GCC 9.2.0 fails to compile the following C++ code (a minimal example so far):

#include <sys/builtin.h>

typedef struct 
{
  signed char volatile done;
  signed char volatile started;
} foo_t;

static inline int
foo (foo_t *once, void (*func) (void))
{
  if (__cxchg (&once->started, 1) == 0)
    {
      func ();
      __cxchg (&once->done, 1);
    }
  do {}  while (!once->done);
  return 0;
}

template<typename _Callable>
void bar(foo_t& __once, _Callable&& __f)
{
  foo(&__once, __f);
}

template void bar(foo_t&, void (*&&)());

using the following command:

g++ -O2 -std=c++11 -c test.cpp

with the following error message:

test.cpp: In function 'void bar(foo_t&, _Callable&&) [with _Callable = void (*)()]'
test.cpp:25:1: error: unsupported size for integer register!!
   25 | }
      | ^

GCC 4.9.2 from RPM also fails but with a different error:

C:/usr/include/386/builtin.h: Assembler messages:
C:/usr/include/386/builtin.h:18: Error: bad register name `%sil'

The full story is here: https://github.com/bitwiseworks/gcc-os2/issues/2#issuecomment-560478903.

dmik commented 4 years ago

Ironically (given that newer GCC tries to be much better in terms of error message informativeness), the 4.9.2 error turns out to be much more informative. It points exactly to the failing construct which is the following assembly for __cxchg:

  __asm__ __volatile__ ("xchgb %0, %1" : "=m"(*p), "=r"(v) : "1"(v));

Note the second output operand (which also acts like input) which is given as "=r"(v). R there, according to https://gcc.gnu.org/onlinedocs/gcc/Simple-Constraints.html#Simple-Constraints, means any general register (including SI and DI on x86). So what the compiler does in -O2 mode here is takes any unused register and tries to use as a second operand of xchgb. Since the second operand of this particular instruction has a 8-bit size, GCC tries to address the lower 8 bits of the selected register. However, for SI register it is impossible to address its lower 8 bits separately (there is no SIL notation) — hence the error message about the bad register name.

GCC 9.2.0 seems to catch this error much earlier, before supplying the generated assembly to the assembler. While debugging the code in gcc/config/i386/i386.c that spits this error, I spotted that the code generator requests GCC register 4 to be used. On x86, GCC registers are matched to x86 registers using an array like [A, B, C, D, SI, DI, ....]. Register 4 matches SI. Then the code generator notices that there is no 8-bit version of SI (using another helper array) and spits its error about unuspported size.

A fix here is quite obvious: restrict GCC inline assembly to only using registers whose lower 8-bits may be accessed directly. On x86 these are A, B, C and D. And the GCC constraint for this set is q (see https://gcc.gnu.org/onlinedocs/gcc/Machine-Constraints.html#Machine-Constraints). So, using the following definition:

  __asm__ __volatile__ ("xchgb %0, %1" : "=m"(*p), "=q"(v) : "1"(v));

fixes the compilation errors both on GCC 4.9.2 and GCC 9.2.0. Congrats.

It's not the end of the story though. First, this patch belongs to LIBC. So it should be fixed there (I will deal with that tomorrow). Second, it rang some bell so I searched for SIL in the @psmedley's GCC repo and found this comment: https://github.com/psmedley/gcc/issues/8#issuecomment-70549405. I have no idea why Paul commented on that (doesn't seem to be related to the issue or any other comment over there) but apparently he seemed to run across this very same issue many years ago. However, as he didn't supply his patch to the right project, it was simply lost.

Anyway, this particular problem seems to have gone now and it's very good.

dmik commented 4 years ago

Closing this as fixed by the LIBC fix.