gcc version 4.8.0 20120510 (experimental) (GCC) #include <immintrin.h> main() { int flag = -1; unsigned status; if ((status = _xbegin()) == _XBEGIN_STARTED) { flag = _xtest(); _xend(); } else printf("abort %x\n", status); printf("%d\n" , flag); printf("%d\n", _xtest()); } gives xtest.c: In function 'main': xtest.c:12:3: warning: incompatible implicit declaration of built-in function 'printf' [enabled by default] printf("abort %x\n", status); ^ xtest.c:15:1: error: unable to generate reloads for: } ^ (jump_insn 7 6 8 2 (parallel [ (set (pc) (if_then_else (ne (unspec [ (const_int 0 [0]) ] UNSPEC_XBEGIN_ABORT) (const_int 0 [0])) (label_ref 8) (pc))) (set (reg:SI 4 si [63]) (unspec_volatile:SI [ (reg:SI 3 bx [64]) ] UNSPECV_XBEGIN)) ]) /pkg/gcc-4.8-120510/lib64/gcc/x86_64-unknown-linux-gnu/4.8.0/include/rtmintrin.h:50 978 {xbegin_1} (nil) -> 8) xtest.c:15:1: internal compiler error: in find_reloads, at reload.c:3825
Hmm it goes away when i remove the (status = _xbegin) so it may be actually xbegin not xtest
(In reply to comment #1) > Hmm it goes away when i remove the (status = _xbegin) > so it may be actually xbegin not xtest Yes, it is xbegin_1 insn pattern that doesn't get registers allocated. However, the pattern (although a bit unusual), looks correct to me.
Reload does not know how to handle reloading into jump instructions.
(define_expand "xbegin" [(set (match_operand:SI 0 "register_operand") (unspec_volatile:SI [(match_dup 1)] UNSPECV_XBEGIN))] "TARGET_RTM" { rtx label = gen_label_rtx (); operands[0] = force_reg (SImode, constm1_rtx); emit_jump_insn (gen_xbegin_1 (operands[0], label)); emit_label (label); LABEL_NUSES (label) = 1; DONE; }) (define_insn "xbegin_1" [(set (pc) (if_then_else (ne (unspec [(const_int 0)] UNSPEC_XBEGIN_ABORT) (const_int 0)) (label_ref (match_operand 1)) (pc))) (set (match_operand:SI 0 "register_operand" "+a") (unspec_volatile:SI [(match_dup 0)] UNSPECV_XBEGIN))] "TARGET_RTM" "xbegin\t%l2" [(set_attr "type" "other") (set_attr "length" "6")]) I think this is the one of the few cases where you want to use + in the constraint and match_dup.
Created attachment 27370 [details] A patch
(In reply to comment #5) > Created attachment 27370 [details] > A patch Patch looks OK to me, but please let Andi play with this a bit, so we are sure we won't hit some other reload limitation with this pattern.
As discussed in PR 53291, please also add a runtime testcase that will cover PR 53291 as well as this PR.
I tested HJs fix on the test case and also on a more complex program and it all works as expected. Please commit.
Sorry I was wrong earlier. Retested now fully with a full test case and HJs patch and i always get aborts The xbegin gets miscompiled now, the in transaction branch disappears. 400460: 48 83 ec 08 sub $0x8,%rsp 400464: b8 ff ff ff ff mov $0xffffffff,%eax 400469: c7 f8 00 00 00 00 xbeginq 40046f <main+0xf> 40046f: bf d8 06 40 00 mov $0x4006d8,%edi 400474: 31 f6 xor %esi,%esi 400476: 31 c0 xor %eax,%eax 400478: e8 b3 ff ff ff callq 400430 <printf@plt> 40047d: 31 ff xor %edi,%edi 40047f: e8 bc ff ff ff callq 400440 <exit@plt> /* PR53315 and PR53291 */ /* { dg-do run } */ /* { dg-options "-O2 -mrtm" } */ #include <immintrin.h> #include <cpuid.h> #include <stdlib.h> #include <stdio.h> static int cpu_has_rtm(void) { if (__get_cpuid_max(0, NULL) >= 7) { unsigned a, b, c, d; __cpuid_count(7, 0, a, b, c, d); return !!(b & bit_RTM); } return 0; } int main(void) { int flag = -1; unsigned status; if (!cpu_has_rtm) { printf("no tsx support. untested\n"); exit(0); } if ((status = _xbegin()) == _XBEGIN_STARTED) { flag = _xtest(); _xend(); } else { /* Note this is legal according to the TSX spec */ printf("unexpected abort %x. untested\n", status); exit(0); } if (flag != 1) abort(); if (_xtest() != 0) abort(); return 0; }
please unsubscribe -----Original Message----- From: andi-gcc at firstfloor dot org Sent: Friday, May 11, 2012 11:35 PM To: gcc-bugs@gcc.gnu.org Subject: [Bug target/53315] simple xtest program generates ICE http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53315 --- Comment #9 from Andi Kleen <andi-gcc at firstfloor dot org> 2012-05-11 21:35:47 UTC --- Sorry I was wrong earlier. Retested now fully with a full test case and HJs patch and i always get aborts The xbegin gets miscompiled now, the in transaction branch disappears. 400460: 48 83 ec 08 sub $0x8,%rsp 400464: b8 ff ff ff ff mov $0xffffffff,%eax 400469: c7 f8 00 00 00 00 xbeginq 40046f <main+0xf> 40046f: bf d8 06 40 00 mov $0x4006d8,%edi 400474: 31 f6 xor %esi,%esi 400476: 31 c0 xor %eax,%eax 400478: e8 b3 ff ff ff callq 400430 <printf@plt> 40047d: 31 ff xor %edi,%edi 40047f: e8 bc ff ff ff callq 400440 <exit@plt> /* PR53315 and PR53291 */ /* { dg-do run } */ /* { dg-options "-O2 -mrtm" } */ #include <immintrin.h> #include <cpuid.h> #include <stdlib.h> #include <stdio.h> static int cpu_has_rtm(void) { if (__get_cpuid_max(0, NULL) >= 7) { unsigned a, b, c, d; __cpuid_count(7, 0, a, b, c, d); return !!(b & bit_RTM); } return 0; } int main(void) { int flag = -1; unsigned status; if (!cpu_has_rtm) { printf("no tsx support. untested\n"); exit(0); } if ((status = _xbegin()) == _XBEGIN_STARTED) { flag = _xtest(); _xend(); } else { /* Note this is legal according to the TSX spec */ printf("unexpected abort %x. untested\n", status); exit(0); } if (flag != 1) abort(); if (_xtest() != 0) abort(); return 0; }
Created attachment 27385 [details] gcc48-pr53315.patch That is because the patch is buggy. Fixed thusly, though haven't tested it on Haswell (obviously) nor sim. Note, it would be nice to have a peephole or something similar (guess peepholes won't do anything across multiple bbs, perhaps machine reorg) to optimize that movl $-1, %eax xbegin .L2 .L2: cmpl $-1, %eax jne .L3 xorl %eax, %eax into say movl $-1, %eax xbegin .L3 xorl %eax, %eax or even xbegin .L3 xorl %eax, %eax
(In reply to comment #11) > Created attachment 27385 [details] > gcc48-pr53315.patch Please introduce check-rtm.h header for use in runtime testcases, as is the case with i.e. check-sse2.h and other feature check headers.
Created attachment 27387 [details] gcc48-pr53315.patch Like this?
I can confirm the simple test program works correctly with Jakub's patch. I'll leave full bootstrap to HJ.
Oh yes and it would be really nice to have those peepholes for xbegin Jakub. I normally use my own macros with asm goto to avoid the ugly code. Do you think machine reorg could be done without slowing down the compiler?
(In reply to comment #13) > Like this? Yes, this is OK, after someone confirms that the testcase works as expected on HW or simulator.
(In reply to comment #15) > Do you think machine reorg could be done without slowing down the compiler? Yes, xbegin RTM pattern can raise a flag that triggers machine reorg (so it won't fire for functions that don't emit xbegin).
Author: jakub Date: Mon May 14 18:47:05 2012 New Revision: 187477 URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=187477 Log: 2012-05-14 Andrew Pinski <apinski@cavium.com> H.J. Lu <hongjiu.lu@intel.com> Jakub Jelinek <jakub@redhat.com> PR target/53315 * config/i386/i386.md (xbegin_1): Use + in constraint and match_dup. (xbegin): Updated. 2012-05-14 Andi Kleen <ak@linux.intel.com> Jakub Jelinek <jakub@redhat.com> PR target/53315 * gcc.target/i386/i386.exp (check_effective_target_rtm): New. * gcc.target/i386/rtm-check.h: New file. * gcc.target/i386/pr53315.c: New test. Added: trunk/gcc/testsuite/gcc.target/i386/pr53315.c trunk/gcc/testsuite/gcc.target/i386/rtm-check.h Modified: trunk/gcc/ChangeLog trunk/gcc/config/i386/i386.md trunk/gcc/testsuite/ChangeLog trunk/gcc/testsuite/gcc.target/i386/i386.exp
Author: eraman Date: Wed Aug 22 21:07:30 2012 New Revision: 190601 URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=190601 Log: 2012-08-22 Easwaran Raman <eraman@google.com> Backport r187387 and r187477 from trunk. r187387: 2012-05-11 Uros Bizjak <ubizjak@gmail.com> PR target/53291 * config/i386/i386.md (xtest): Use NE condition in ix86_expand_setcc. r187477: 2012-05-14 Andrew Pinski <apinski@cavium.com> H.J. Lu <hongjiu.lu@intel.com> Jakub Jelinek <jakub@redhat.com> PR target/53315 * config/i386/i386.md (xbegin_1): Use + in constraint and match_dup. (xbegin): Updated. gcc/testsuite/ChangeLog.google-4_7: 2012-08-22 Easwaran Raman <eraman@google.com> Backport r187477 from trunk: 2012-05-14 Andi Kleen <ak@linux.intel.com> Jakub Jelinek <jakub@redhat.com> PR target/53315 * gcc.target/i386/i386.exp (check_effective_target_rtm): New. * gcc.target/i386/rtm-check.h: New file. * gcc.target/i386/pr53315.c: New test. Added: branches/google/gcc-4_7/gcc/testsuite/gcc.target/i386/pr53315.c - copied unchanged from r187477, trunk/gcc/testsuite/gcc.target/i386/pr53315.c branches/google/gcc-4_7/gcc/testsuite/gcc.target/i386/rtm-check.h - copied unchanged from r187477, trunk/gcc/testsuite/gcc.target/i386/rtm-check.h Modified: branches/google/gcc-4_7/gcc/ChangeLog.google-4_7 branches/google/gcc-4_7/gcc/config/i386/i386.md branches/google/gcc-4_7/gcc/testsuite/ChangeLog.google-4_7 branches/google/gcc-4_7/gcc/testsuite/gcc.target/i386/i386.exp
Fixed for some time