[Bug target/64015] [5.0 Regression] AArch64 ICE due to conditional compare

[Bug target/64015] [5.0 Regression] AArch64 ICE due to conditional compare

[ktkachov at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64015

ktkachov at gcc dot gnu.org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
      Known to work|                            |4.8.4, 4.9.2
   Target Milestone|---                         |5.0
      Known to fail|                            |5.0

[Bug target/64015] New: [5.0 Regression] AArch64 ICE due to conditional compare

[ktkachov at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64015

            Bug ID: 64015
           Summary: [5.0 Regression] AArch64 ICE due to conditional
                    compare
           Product: gcc
           Version: 5.0
            Status: UNCONFIRMED
          Keywords: ice-on-valid-code
          Severity: normal
          Priority: P3
         Component: target
          Assignee: unassigned at gcc dot gnu.org
          Reporter: ktkachov at gcc dot gnu.org
                CC: zhenqiang.chen at arm dot com
            Target: aarch64*

Created attachment 34065
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=34065&action=edit
Reduced testcase

Attached testcase ICEs with:

ice.c: In function 'test':
ice.c:4:1: error: unrecognizable insn:
 }
 ^
(insn 8 7 9 2 (set (reg:CC_DGTU 66 cc)
        (compare:CC_DGTU (and:SI (ne:SI (reg:CC_DGTU 66 cc)
                    (const_int 0 [0]))
                (gtu:SI (reg/v:SI 79 [ b ])
                    (const_int 252 [0xfc])))
            (const_int 0 [0]))) ice.c:3 -1
     (nil))


I've tracked it down to the aarch64 conditional compare patches. r217638 works
whereas r217646 has the ICE.

-O2 should be enough to reproduce.
Zhenqiang, can you take a look please?

[Bug target/64015] [5.0 Regression] AArch64 ICE due to conditional compare

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64015

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Priority|P3                          |P1

[Bug target/64015] [5.0 Regression] AArch64 ICE due to conditional compare

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64015

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |ASSIGNED
   Last reconfirmed|                            |2014-11-21
     Ever confirmed|0                           |1

--- Comment #1 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
I have a fix which I will submit this weekend.
With my fix we produce:
    uxtb    w0, w0
    uxtb    w1, w1
    cmp    w0, 10
    mov    w0, 252
    ccmp    w1, w0, 0, hi
    cset    w0, hi

[Bug target/64015] [5.0 Regression] AArch64 ICE due to conditional compare

[zhenqiang.chen at arm dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64015

--- Comment #2 from Zhenqiang Chen <zhenqiang.chen at arm dot com> ---
You force it to register? In fact, I tend to not force it to register in
gen_ccmp_next, since it will introduce more overhead for ccmp, which
performance maybe worse.

My patch to fix the issue is at:
https://gcc.gnu.org/ml/gcc-patches/2014-11/msg02966.html

For CCMP, we still miss two optimizations for it:
1) Change the order of compares. In the case, if you change it to

  b > 252 && a > 10

You don't need "mov w0, 252"

        uxtb    w1, w1
    uxtb    w0, w0
    cmp    w1, 252
    ccmp    w0, 10, 0, hi
    cset    w0, hi
    ret

2) How to justify it is valueable (the overhead of ccmp is OK) when generating
ccmp?

[Bug target/64015] [5.0 Regression] AArch64 ICE due to conditional compare

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64015

--- Comment #3 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
(In reply to Zhenqiang Chen from comment #2)
> 2) How to justify it is valueable (the overhead of ccmp is OK) when
> generating ccmp?

If we ignore the case for swapping.
Try this one:
int
test (int a, int b)
{
  return (a > 252) && b > 252;
}

With my patch to do the forcing:
test:
        cmp     w0, 252
        mov     w0, 252
        ccmp    w1, w0, 4, gt
        cset    w0, gt
        ret

Without:
test:
        cmp     w0, 252
        cset    w2, gt
        cmp     w1, 252
        cset    w0, gt
        and     w0, w2, w0
        ret

Or better yet take:
int
test (int a, int b)
{
  return (a > 321223) && b > 321224;
}
Without:
test:
        mov     w3, 59079
        mov     w2, 59080
        movk    w3, 0x4, lsl 16
        movk    w2, 0x4, lsl 16
        cmp     w0, w3
        cset    w3, gt
        cmp     w1, w2
        cset    w0, gt
        and     w0, w3, w0
        ret

With forcing:
test:
        mov     w3, 59079
        mov     w2, 59080
        movk    w3, 0x4, lsl 16
        movk    w2, 0x4, lsl 16
        cmp     w0, w3
        ccmp    w1, w2, 4, gt
        cset    w0, gt
        ret

--- CUT ---
Also take:
int
test (int a, int b)
{
  return (a > 33) && b > 33;
}
Without:
test:
        cmp     w0, 33
        cset    w2, gt
        cmp     w1, 33
        cset    w0, gt
        and     w0, w2, w0
        ret
With forcing:
test:
        cmp     w0, 33
        mov     w0, 33
        ccmp    w1, w0, 4, gt
        cset    w0, gt
        ret


See how with forcing is always the same size or smaller?

[Bug target/64015] [5.0 Regression] AArch64 ICE due to conditional compare

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64015

--- Comment #4 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
(In reply to Andrew Pinski from comment #3)
> See how with forcing is always the same size or smaller?

Actually is always smaller by at least one instruction. due to the need to  do
one extra cset and one and/or compared to one mov.

[Bug target/64015] [5.0 Regression] AArch64 ICE due to conditional compare

[zhenqiang.chen at arm dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64015

--- Comment #5 from Zhenqiang Chen <zhenqiang.chen at arm dot com> ---
It seams you always win with ccmp. Please go ahead for your patch and make sure
the following case work.

int
test (unsigned short a, unsigned char b)
{
  return a > 0xfff2 && b > 252;
}

Thanks!
-Zhenqiang

[Bug target/64015] [5.0 Regression] AArch64 ICE due to conditional compare

[ktkachov at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64015

--- Comment #6 from ktkachov at gcc dot gnu.org ---
By the way, this ICE manifests when building perlbmk in SPEC2006

[Bug target/64015] [5.0 Regression] AArch64 ICE due to conditional compare

[zhenqiang.chen at arm dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64015

--- Comment #7 from Zhenqiang Chen <zhenqiang.chen at arm dot com> ---
Sorry for blocking your benchmark tests. I had reverted the ccmp patch. 

I will rework the patch based on Richard Henderson's comments:
https://gcc.gnu.org/ml/gcc-patches/2014-11/msg03100.html

[Bug target/64015] [5.0 Regression] AArch64 ICE due to conditional compare

[StaffLeavers at arm dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64015

--- Comment #8 from StaffLeavers at arm dot com ---
zhenqiang.chen no longer works for ARM.

Your email will be forwarded to their line manager.


Please do not reply to this email.
If you need more information, please email real-postmaster@arm.com

Thank you.

[Bug target/64015] [5.0 Regression] AArch64 ICE due to conditional compare

[StaffLeavers at arm dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64015

--- Comment #10 from StaffLeavers at arm dot com ---
zhenqiang.chen no longer works for ARM.

Your email will be forwarded to their line manager.


Please do not reply to this email.
If you need more information, please email real-postmaster@arm.com

Thank you.

[Bug target/64015] [5.0 Regression] AArch64 ICE due to conditional compare

[StaffLeavers at arm dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64015

--- Comment #9 from StaffLeavers at arm dot com ---
zhenqiang.chen no longer works for ARM.

Your email will be forwarded to their line manager.


Please do not reply to this email.
If you need more information, please email real-postmaster@arm.com

Thank you.

[Bug target/64015] [5.0 Regression] AArch64 ICE due to conditional compare

[StaffLeavers at arm dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64015

--- Comment #11 from StaffLeavers at arm dot com ---
zhenqiang.chen no longer works for ARM.

Your email will be forwarded to their line manager.


Please do not reply to this email.
If you need more information, please email real-postmaster@arm.com

Thank you.

[Bug target/64015] [5.0 Regression] AArch64 ICE due to conditional compare

[StaffLeavers at arm dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64015

--- Comment #12 from StaffLeavers at arm dot com ---
zhenqiang.chen no longer works for ARM.

Your email will be forwarded to their line manager.


Please do not reply to this email.
If you need more information, please email real-postmaster@arm.com

Thank you.

[Bug target/64015] [5.0 Regression] AArch64 ICE due to conditional compare

[StaffLeavers at arm dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64015

--- Comment #13 from StaffLeavers at arm dot com ---
zhenqiang.chen no longer works for ARM.

Your email will be forwarded to their line manager.


Please do not reply to this email.
If you need more information, please email real-postmaster@arm.com

Thank you.

[Bug target/64015] [5.0 Regression] AArch64 ICE due to conditional compare

[StaffLeavers at arm dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64015

--- Comment #14 from StaffLeavers at arm dot com ---
zhenqiang.chen no longer works for ARM.

Your email will be forwarded to their line manager.


Please do not reply to this email.
If you need more information, please email real-postmaster@arm.com

Thank you.

[Bug target/64015] [5.0 Regression] AArch64 ICE due to conditional compare

[StaffLeavers at arm dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64015

--- Comment #15 from StaffLeavers at arm dot com ---
zhenqiang.chen no longer works for ARM.

Your email will be forwarded to their line manager.


Please do not reply to this email.
If you need more information, please email real-postmaster@arm.com

Thank you.

[Bug target/64015] [5.0 Regression] AArch64 ICE due to conditional compare

[jiwang at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64015

--- Comment #16 from Jiong Wang <jiwang at gcc dot gnu.org> ---
Author: jiwang
Date: Fri Jan 16 11:48:00 2015
New Revision: 219723

URL: https://gcc.gnu.org/viewcvs?rev=219723&root=gcc&view=rev
Log:
[AArch64] Enable CCMP support for AArch64, PR64015 resolved

gcc/
2015-01-16  Zhenqiang Chen  <zhenqiang.chen@arm.com>

    PR target/64015
    * ccmp.c (expand_ccmp_next): New function.
    (expand_ccmp_expr_1, expand_ccmp_expr): Handle operand insn sequence
    and compare insn sequence.
    * config/aarch64/aarch64.c (aarch64_code_to_ccmode,
    aarch64_gen_ccmp_first, aarch64_gen_ccmp_next): New functions.
    (TARGET_GEN_CCMP_FIRST, TARGET_GEN_CCMP_NEXT): New MICRO.
    * config/aarch64/aarch64.md (*ccmp_and): Changed to ccmp_and<mode>.
    (*ccmp_ior): Changed to ccmp_ior<mode>.
    (cmp<mode>): New pattern.
    * doc/tm.texi (TARGET_GEN_CCMP_FIRST, TARGET_GEN_CCMP_NEXT): Update
    parameters.
    * target.def (gen_ccmp_first, gen_ccmp_next): Update parameters.

gcc/testsuite/
2015-01-16  Zhenqiang Chen <zhenqiang.chen@arm.com>

    * gcc.dg/pr64015.c: New test.


Added:
    trunk/gcc/testsuite/gcc.dg/pr64015.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/ccmp.c
    trunk/gcc/config/aarch64/aarch64.c
    trunk/gcc/config/aarch64/aarch64.md
    trunk/gcc/doc/tm.texi
    trunk/gcc/target.def
    trunk/gcc/testsuite/ChangeLog

[Bug target/64015] [5.0 Regression] AArch64 ICE due to conditional compare

[jiwang at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64015

Jiong Wang <jiwang at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
         Resolution|---                         |FIXED

--- Comment #17 from Jiong Wang <jiwang at gcc dot gnu.org> ---
mark as fixed.