Bug 80115 - [7 Regression] OpenJDK 1.8 fails to build
Summary: [7 Regression] OpenJDK 1.8 fails to build
Alias: None
Product: gcc
Classification: Unclassified
Component: target (show other bugs)
Version: 7.0.1
: P3 normal
Target Milestone: 7.0
Assignee: Not yet assigned to anyone
Keywords: rejects-valid
Depends on:
Reported: 2017-03-20 11:29 UTC by Richard Biener
Modified: 2017-03-22 06:06 UTC (History)
4 users (show)

See Also:
Target: i?86-*-*
Known to work:
Known to fail:
Last reconfirmed: 2017-03-20 00:00:00

unreduced testcase (498.51 KB, application/x-xz)
2017-03-20 11:31 UTC, Richard Biener

Note You need to log in before you can comment on or make changes to this bug.
Description Richard Biener 2017-03-20 11:29:48 UTC
g++-7 /tmp/defNewGeneration.ii -S -m32 -O2 -w
/home/abuild/rpmbuild/BUILD/icedtea-3.3.0/openjdk-boot/hotspot/src/share/vm/memory/defNewGeneration.cpp: In member function ‘virtual void DefNewGeneration::collect(bool, bool, size_t, bool)’:
/home/abuild/rpmbuild/BUILD/icedtea-3.3.0/openjdk-boot/hotspot/src/share/vm/memory/defNewGeneration.cpp:731:1: error: unsupported size for integer register

#1  0x00000000015a3fa9 in print_reg (x=0x7fffee20f180, code=0, file=0x2b9fbf0)
    at /space/rguenther/src/svn/gcc-7-branch/gcc/config/i386/i386.c:17667
17667           error ("unsupported size for integer register");
(gdb) l
17662         break;
17663       case 1:
17664         if (regno >= ARRAY_SIZE (qi_reg_name))
17665           goto normal;
17666         if (!ANY_QI_REGNO_P (regno))
17667           error ("unsupported size for integer register");
(gdb) p regno
$1 = 4
(gdb) p qi_reg_name[4]
$3 = 0x1f26494 "sil"

the asm in question is

(insn:TI 14 734 15 2 (parallel [
            (asm_operands/v ("990: nop
.pushsection .note.stapsdt,"?","note"
.balign 4
.4byte 992f-991f,994f-993f,3
991: .asciz "stapsdt"
992: .balign 4
993: .4byte 990b
.4byte _.stapsdt.base
.4byte 0
.asciz "hotspot"
.asciz "gc__collection__defnew__begin"
.asciz "%n0@%1 %n2@%3 %n4@%5 %n6@%7"
994: .balign 4
") ("") 0 [
                    (const_int -1 [0xffffffffffffffff])
                    (reg:QI 0 ax [orig:200 full ] [200])
                    (const_int -1 [0xffffffffffffffff])
                    (reg:QI 2 cx [orig:202 clear_all_soft_refs ] [202])
                    (const_int -4 [0xfffffffffffffffc])
                    (mem/c:SI (plus:SI (reg/f:SI 6 bp)
                            (const_int 20 [0x14])) [3 size+0 S4 A32])
                    (const_int -1 [0xffffffffffffffff])
                    (reg:QI 4 si [orig:205 is_tlab ] [205])
                    (asm_input:SI ("n") /home/abuild/rpmbuild/BUILD/icedtea-3.3.0/openjdk-boot/hotspot/src/share/vm/memory/defNewGeneration.cpp:571)
                    (asm_input:QI ("nor") /home/abuild/rpmbuild/BUILD/icedtea-3.3.0/openjdk-boot/hotspot/src/share/vm/memory/defNewGeneration.cpp:571)
                    (asm_input:SI ("n") /home/abuild/rpmbuild/BUILD/icedtea-3.3.0/openjdk-boot/hotspot/src/share/vm/memory/defNewGeneration.cpp:571)
                    (asm_input:QI ("nor") /home/abuild/rpmbuild/BUILD/icedtea-3.3.0/openjdk-boot/hotspot/src/share/vm/memory/defNewGeneration.cpp:571)
                    (asm_input:SI ("n") /home/abuild/rpmbuild/BUILD/icedtea-3.3.0/openjdk-boot/hotspot/src/share/vm/memory/defNewGeneration.cpp:571)
                    (asm_input:SI ("nor") /home/abuild/rpmbuild/BUILD/icedtea-3.3.0/openjdk-boot/hotspot/src/share/vm/memory/defNewGeneration.cpp:571)
                    (asm_input:SI ("n") /home/abuild/rpmbuild/BUILD/icedtea-3.3.0/openjdk-boot/hotspot/src/share/vm/memory/defNewGeneration.cpp:571)
                    (asm_input:QI ("nor") /home/abuild/rpmbuild/BUILD/icedtea-3.3.0/openjdk-boot/hotspot/src/share/vm/memory/defNewGeneration.cpp:571)
                 [] /home/abuild/rpmbuild/BUILD/icedtea-3.3.0/openjdk-boot/hotspot/src/share/vm/memory/defNewGeneration.cpp:571)
            (clobber (reg:CCFP 18 fpsr))
            (clobber (reg:CC 17 flags))
        ]) "/home/abuild/rpmbuild/BUILD/icedtea-3.3.0/openjdk-boot/hotspot/src/share/vm/memory/defNewGeneration.cpp":571 -1
     (expr_list:REG_DEAD (reg:QI 4 si [orig:205 is_tlab ] [205])
        (expr_list:REG_DEAD (reg:QI 2 cx [orig:202 clear_all_soft_refs ] [202])
            (expr_list:REG_DEAD (reg:QI 0 ax [orig:200 full ] [200])
                (expr_list:REG_UNUSED (reg:CCFP 18 fpsr)
                    (expr_list:REG_UNUSED (reg:CC 17 flags)

note that it builds successfully with -O1 or -O0.  The same issue happens
in multiple places in OpenJDK 1.8 and stems from their HS_DTRACE_PROBE4
macro expansion which eventually expands to STAP_PROBE4 / _SDT_PROBE which
is defined as

# 33 "/usr/include/sys/sdt.h" 3 4
#define _SDT_PROBE(provider,name,n,arglist) do { __asm__ __volatile__ (_SDT_ASM_BODY(provider, name, _SDT_ASM_ARGS, (n)) :: _SDT_ASM_OPERANDS_ ##n arglist); __asm__ __volatile__ (_SDT_ASM_BASE); } while (0)


#define _SDT_ASM_OPERANDS_4(arg1,arg2,arg3,arg4) _SDT_ASM_OPERANDS_3(arg1, arg2, arg3), _SDT_ARG(4, arg4)

(ick, all heavily macroized stuff)

#define _SDT_ARG(n,x) [_SDT_S ##n] "n" ((_SDT_ARGSIGNED (x) ? 1 : -1) * (int) _SDT_ARGSIZE (x)), [_SDT_A ##n] _SDT_ARG_CONSTRAINT_STRING (STAP_SDT_ARG_CONSTRAINT) (_SDT_ARGVAL (x))


I'll note that we accept (reg:QI 4 si ...) just fine.
Comment 1 Richard Biener 2017-03-20 11:31:26 UTC
Created attachment 41003 [details]
unreduced testcase
Comment 2 Markus Trippelsdorf 2017-03-20 12:13:33 UTC
struct A {
  void m_fn1();
  void m_fn2(bool, bool);
void A::m_fn2(bool p1, bool p2) {
      ".asciz \"%n[_SDT_S1]@%[_SDT_A1] @%[_SDT_A2] @%[_SDT_A3] @%[_SDT_A4]\"" ::
          [_SDT_S1] "n"(0),
      [_SDT_A1] "n"(0), [_SDT_A2] "nor"(p1), [_SDT_A3] "n"(0),
      [_SDT_A4] "nor"(p2));
Comment 3 Richard Biener 2017-03-20 13:14:02 UTC
This is from systemtap 3.0, didn't try with 3.1 which seems to be available since a few weeks.
Comment 4 Jakub Jelinek 2017-03-21 11:39:48 UTC
That looks like a systemtap or OpenJDK bug to me.
For bool and other 8-bit values, one can't use the "r" constraint on ia32, but has to use "q" constraint instead, because ia32 32-bit doesn't have %sil, %dil, %bpl and %spl registers.
So, for #ifdef __i386__, either sdt.h needs to force the sub-int arguments into int (e.g.
#define _SDT_ARGVAL(x) ((x) + 0)
would do it I think), or
# define STAP_SDT_ARG_CONSTRAINT        noq
instead of nor (again, ia32 only), or perhaps use the constraint conditional depending on sizeof (x).
This "worked" in gcc 6 and earlier because we happily emitted %sil etc. into the inline assembly, even when it is not valid for 32-bit code, but starting with
r245815 we diagnose that.
Comment 5 Frank Ch. Eigler 2017-03-21 13:45:37 UTC
(In reply to Jakub Jelinek from comment #4)
> This "worked" in gcc 6 and earlier because we happily emitted %sil etc. into
> the inline assembly, even when it is not valid for 32-bit code, but starting
> with r245815 we diagnose that.

Just curious, but could the "r" constraint be reinterpreted by gcc>6 so that it emits %si etc. for these small values rather than %sil?
Comment 6 Jakub Jelinek 2017-03-21 13:58:52 UTC
%si is 16-bit register name, not 8-bit register name.  For that you need to use 16-bit operand, not 8-bit.
Another option is to use a modifier to force some other size.
I don't know what the stap note parser actually cares about, if it doesn't make any difference between %al, %ax, %eax and grabs the argument size from somewhere else, you might e.g.
#define _SDT_ARGTMPL(id) %k[id]
instead of %[id]
for defined(__i386__) and then you'll always get %eax, %esi, etc. regardless of whether the argument is 8-bit, 16-bit, 32-bit or 64-bit.
Or you can use %w[id] to always get %ax, %si, etc. (shorter).
Comment 7 Frank Ch. Eigler 2017-03-21 14:22:55 UTC
The systemtap operand encoding machinery separately gives us the byte-size of the operand, so even if gcc told us %si, we'd only look at %sil only anyway.  But if gcc cannot let that level of ambiguity be, then I guess we must work around the change.

In the sdt.h, we enjoyed using machine-independent operant-constraint codes - until now, I guess.
Comment 8 Jakub Jelinek 2017-03-21 14:27:22 UTC
Then do:

--- /usr/include/sys/sdt.h	2017-01-25 23:20:05.000000000 +0100
+++ /usr/include/sys/sdt.h	2017-03-21 15:26:14.448999404 +0100
@@ -173,6 +173,8 @@ __extension__ extern unsigned long long
 #if defined __powerpc__ || defined __powerpc64__
 # define _SDT_ARGTMPL(id)	%I[id]%[id]
+#elif defined __i386__
+# define _SDT_ARGTMPL(id)	%w[id]
 # define _SDT_ARGTMPL(id)	%[id]

and it ought to work right (and as added bonus be more compact (shorter strings)).
Comment 9 Frank Ch. Eigler 2017-03-21 22:05:47 UTC
Thanks, Jakub; git systemtap now includes your %w[] patch.