Bug 67328 - range test rather than single bit test for code testing enum values
Summary: range test rather than single bit test for code testing enum values
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: tree-optimization (show other bugs)
Version: 5.1.1
: P3 enhancement
Target Milestone: ---
Assignee: Not yet assigned to anyone
URL:
Keywords: missed-optimization
Depends on:
Blocks:
 
Reported: 2015-08-23 13:41 UTC by Alan Modra
Modified: 2017-06-23 04:19 UTC (History)
4 users (show)

See Also:
Host:
Target: x86_64-linux, powerpc64le-linux
Build:
Known to work:
Known to fail:
Last reconfirmed: 2016-09-12 00:00:00


Attachments
testcase from binutils/include/bfdlink.h (277 bytes, text/plain)
2015-08-23 13:45 UTC, Alan Modra
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Alan Modra 2015-08-23 13:41:38 UTC
The attached file generates ideal code, a test of a single bit, for the condition in test_pic or test_exe depending on -DALT.  gcc ought to be able to optimise to a single bit test for both functions whether or not -DALT.  It looks like gcc incorrectly prefers a range test.
Comment 1 Alan Modra 2015-08-23 13:45:36 UTC
Created attachment 36247 [details]
testcase from binutils/include/bfdlink.h
Comment 2 Richard Biener 2015-08-25 08:38:29 UTC
Note this is already fold-const.c:optimize_bit_field_compare at work.  With -DALT (non-working code) we get

;; Function test_pic (null)
;; enabled by -tree-original


{
  if ((BIT_FIELD_REF <*info, 8, 0> & 3) + 254 <= 1)

and

;; Function test_exe (null)
;; enabled by -tree-original


{
  if ((SAVE_EXPR <BIT_FIELD_REF <*info, 8, 0> & 3>) == 0 || (SAVE_EXPR <BIT_FIELD_REF <*info, 8, 0> & 3>) == 2)

from it.  Without -DALT

;; Function test_pic (null)
;; enabled by -tree-original


{
  if ((SAVE_EXPR <BIT_FIELD_REF <*info, 8, 0> & 3>) == 3 || (SAVE_EXPR <BIT_FIELD_REF <*info, 8, 0> & 3>) == 1)

;; Function test_exe (null)
;; enabled by -tree-original


{
  if ((BIT_FIELD_REF <*info, 8, 0> & 3) <= 1)


I see more that a single bit test for both cases btw, mostly because we
need to mask the padding.  Not sure what optimal code you expect here.
Comment 3 Alan Modra 2015-08-25 13:09:07 UTC
Maybe adding #if ALT only confused the issue.  For -UALT -O1 on x86_64 I get

test_pic:
        testb   $1, (%rdi)
        je      .L1
        addl    $1, result(%rip)
.L1:
        rep ret

test_exe:
        movzbl  (%rdi), %eax
        andl    $3, %eax
        cmpb    $1, %al
        ja      .L3
        addl    $1, result(%rip)
.L3:
        rep ret

For test_exe I would like to see the following equivalent and better optimised code.

test_exe:
        testb   $2, (%rdi)
        jne     .L3
        addl    $1, result(%rip)
.L3:
        rep ret

The optimisation to a bit test for test_pic happens in reassoc1, or at least that's where the process starts.
Comment 4 Yuri Gribov 2017-01-25 08:07:37 UTC
Proposed patch in https://gcc.gnu.org/ml/gcc-patches/2017-01/msg01936.html
Comment 5 Oleg Endo 2017-01-26 12:53:48 UTC
PR 67731 maybe related or dup?
Comment 6 Yuri Gribov 2017-01-28 22:14:01 UTC
(In reply to Oleg Endo from comment #5)
> PR 67731 maybe related or dup?

Related but not a dup. This bug is caused by frontend folding two bitfield accesses to a single comparison which prevented generation of bit test later in tree-opt.
Comment 7 chefmax 2017-06-13 11:14:26 UTC
Author: chefmax
Date: Tue Jun 13 11:13:52 2017
New Revision: 249149

URL: https://gcc.gnu.org/viewcvs?rev=249149&root=gcc&view=rev
Log:
2017-06-13  Yury Gribov  <tetra2005@gmail.com>

gcc/
	PR tree-optimization/67328
	* fold-const.c (maskable_range_p): New function.
	(build_range_check): Generate bittests if possible.

gcc/testsuite/
	PR tree-optimization/67328
	* c-c++-common/fold-masked-cmp-1.c: New test.
	* c-c++-common/fold-masked-cmp-2.c: Likewise.
	* gcc.dg/pr46309.c: Fix pattern.
	* gcc.dg/pr46309-2.c: Likewise.

Added:
    trunk/gcc/testsuite/c-c++-common/fold-masked-cmp-1.c
    trunk/gcc/testsuite/c-c++-common/fold-masked-cmp-2.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/fold-const.c
    trunk/gcc/testsuite/ChangeLog
    trunk/gcc/testsuite/gcc.dg/pr46309-2.c
    trunk/gcc/testsuite/gcc.dg/pr46309.c
Comment 8 Yuri Gribov 2017-06-20 06:16:24 UTC
Alan,(In reply to Alan Modra from comment #0)
> It looks like gcc incorrectly prefers a range test.

Alan, can we close this?
Comment 9 Alan Modra 2017-06-23 04:19:40 UTC
Thanks for the fix!