Bug 91126 - [7 regression] Incorrect constant propagation of BIT_FIELD_REF
Summary: [7 regression] Incorrect constant propagation of BIT_FIELD_REF
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: tree-optimization (show other bugs)
Version: 10.0
: P2 normal
Target Milestone: 7.5
Assignee: Richard Biener
URL:
Keywords: wrong-code
Depends on:
Blocks:
 
Reported: 2019-07-09 18:00 UTC by Wilco
Modified: 2019-09-02 12:58 UTC (History)
1 user (show)

See Also:
Host:
Target:
Build:
Known to work: 10.0, 4.6.3, 7.4.1, 8.3.1, 9.1.1
Known to fail: 4.7.4, 7.4.0, 8.3.0, 9.1.0
Last reconfirmed: 2019-07-09 00:00:00


Attachments
patch (393 bytes, patch)
2019-07-09 19:03 UTC, Richard Biener
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Wilco 2019-07-09 18:00:41 UTC
gcc.c-torture/execute/pr31448-2.c generates incorrect code due to a recent change with -O2 -mbig-endian on AArch64. fre3 constant-folds a BIT_FIELD_REF <a, 32, 0> with value 0xFEFEFEFE as 0xFFFEFEFE. As a result the if-statement is removed and we abort:

Value numbering store a.iIndex1 to -65794
Setting value number of .MEM_8 to .MEM_8 (changed)
Value numbering stmt = _1 = BIT_FIELD_REF <a, 32, 0>;
Successfully combined 2 partial definitions
Setting value number of _1 to 4294901503 (changed)
Value numbering stmt = _2 = _1 & 4294967040;
Match-and-simplified _1 & 4294967040 to 4294901248
RHS _1 & 4294967040 simplified to 4294901248
Setting value number of _2 to 4294901248 (changed)
Value numbering stmt = if (_2 != 4278124032)
marking known outgoing edge 2 -> 3 executable
Block 1: BB4 found not executable
...

Merging blocks 2 and 3
fix_loop_structure: fixing up loops for function
main ()
{
  struct stD.3411 aD.3421;

;;   basic block 2, loop depth 0, count 0 (precise), probably never executed
;;   Invalid sum of incoming counts 1073741824 (estimated locally), should be 0 (precise)
;;    prev block 0, next block 1, flags: (NEW, REACHABLE, VISITED)
;;    pred:       ENTRY [always]  count:1073741824 (estimated locally) (FALLTHRU,EXECUTABLE)
  # .MEM_4 = VDEF <.MEM_3(D)>
  nextD.3412 = &aD.3421;
  # .MEM_7 = VDEF <.MEM_4>
  aD.3421.iIndexD.3408 = -65794;
  # .MEM_8 = VDEF <.MEM_7>
  aD.3421.iIndex1D.3409 = -65794;
  # .MEM_6 = VDEF <.MEM_8>
  # USE = nonlocal { D.3421 } (escaped)
  # CLB = nonlocal { D.3421 } (escaped)
  abortD.1083 ();
;;    succ:

}
Comment 1 Andrew Pinski 2019-07-09 18:12:53 UTC
BIT_FIELD_INSERT also has similar wrong behavior.
Comment 2 Richard Biener 2019-07-09 18:40:31 UTC
Mine.
Comment 3 Richard Biener 2019-07-09 19:01:16 UTC
So the issue is the usual GET_MODE_SIZE != ref->size.  native_encode_int encodes two 32bit numbers (but only 3 bytes from them, in big-endian) while VN expects it to encode a 3-byte 24bit number.  There's the possibility to disable handling of non-mode precision entities or properly fix it (I think we "simply" need to offset the encoding for big-endian).  Note the issue is probably latent for the other places VN uses native_encode_expr on non-mode precision entities.

There's already a guard on non-mode precision extracts but not for mode-precision extracts of non-mode precision values which I think has the very same issue.
Comment 4 Richard Biener 2019-07-09 19:03:15 UTC
Created attachment 46581 [details]
patch

Untested patch that fixes the issue.

Value numbering stmt = _1 = BIT_FIELD_REF <a, 32, 0>;
Successfully combined 2 partial definitions
Setting value number of _1 to 4278124286 (changed)
Comment 5 Richard Biener 2019-07-09 19:18:46 UTC
Oh, and I think it's a pity we're trying to stay away from optimizing the bitfield cases.  I suppose we could also use encode_tree_to_bitpos from
the store-merging pass for this purpose, but while operating on byte
granularity encodes/interprets only we can use the simple offsetting.
Comment 6 Richard Biener 2019-07-10 10:58:14 UTC
Testcase that fails for a longer time already:

struct S
{
  int a : 24;
  int b : 8;
} s;

int
main()
{
 s.a = 0xfefefe;
 s.b = 0xfe;
 unsigned char c;
 c = ((unsigned char *)&s)[0];
 if (c != 0xfe)
   __builtin_abort ();
 c = ((unsigned char *)&s)[1];
 if (c != 0xfe)
   __builtin_abort ();
 c = ((unsigned char *)&s)[2];
 if (c != 0xfe)
   __builtin_abort ();
 c = ((unsigned char *)&s)[3];
 if (c != 0xfe)
   __builtin_abort ();
 return 0;
}
Comment 7 Richard Biener 2019-07-10 11:04:08 UTC
So the new testcase should fail since GCC 4.7.
Comment 8 Richard Biener 2019-07-10 13:40:25 UTC
Fixed on trunk sofar.
Comment 9 Richard Biener 2019-07-10 13:40:43 UTC
Author: rguenth
Date: Wed Jul 10 13:40:12 2019
New Revision: 273355

URL: https://gcc.gnu.org/viewcvs?rev=273355&root=gcc&view=rev
Log:
2019-07-10  Richard Biener  <rguenther@suse.de>

	PR tree-optimization/91126
	* tree-ssa-sccvn.c (n_walk_cb_data::push_partial_def): Adjust
	native encoding offset for BYTES_BIG_ENDIAN.
	(vn_reference_lookup_3): Likewise.

	* gcc.dg/torture/pr91126.c: New testcase.

Added:
    trunk/gcc/testsuite/gcc.dg/torture/pr91126.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/testsuite/ChangeLog
    trunk/gcc/tree-ssa-sccvn.c
Comment 10 Richard Biener 2019-07-31 15:41:07 UTC
Author: rguenth
Date: Wed Jul 31 15:40:36 2019
New Revision: 273939

URL: https://gcc.gnu.org/viewcvs?rev=273939&root=gcc&view=rev
Log:
2019-07-31  Richard Biener  <rguenther@suse.de>

	Backport from mainline
	2019-07-19  Richard Biener  <rguenther@suse.de>

	PR tree-optimization/91200
	* tree-ssa-phiopt.c (cond_store_replacement): Check we have
	no PHI nodes in middle-bb.

	* gcc.dg/torture/pr91200.c: New testcase.

	2019-07-15  Richard Biener  <rguenther@suse.de>

	PR middle-end/91162
	* tree-cfg.c (move_block_to_fn): When releasing a virtual PHI
	node make sure to replace all uses with something valid.

	* gcc.dg/autopar/pr91162.c: New testcase.

	2019-07-12  Richard Biener  <rguenther@suse.de>

	PR tree-optimization/91145
	* tree-vect-slp.c (vect_build_slp_tree_2): Fix reduction
	chain check.

	* gcc.dg/torture/pr91145.c: New testcase.

	2019-07-11  Richard Biener  <rguenther@suse.de>

	PR middle-end/91131
	* gimplify.c (gimplify_compound_literal_expr): Force a temporary
	when the object is volatile and we have not cleared it even though
	there are no nonzero elements.

	* gcc.target/i386/pr91131.c: New testcase.

	2019-07-10  Richard Biener  <rguenther@suse.de>

	PR tree-optimization/91126
	* tree-ssa-sccvn.c (n_walk_cb_data::push_partial_def): Adjust
	native encoding offset for BYTES_BIG_ENDIAN.
	(vn_reference_lookup_3): Likewise.

	* gcc.dg/torture/pr91126.c: New testcase.

Added:
    branches/gcc-9-branch/gcc/testsuite/gcc.dg/autopar/pr91162.c
    branches/gcc-9-branch/gcc/testsuite/gcc.dg/torture/pr91126.c
    branches/gcc-9-branch/gcc/testsuite/gcc.dg/torture/pr91145.c
    branches/gcc-9-branch/gcc/testsuite/gcc.dg/torture/pr91200.c
    branches/gcc-9-branch/gcc/testsuite/gcc.target/i386/pr91131.c
Modified:
    branches/gcc-9-branch/gcc/ChangeLog
    branches/gcc-9-branch/gcc/gimplify.c
    branches/gcc-9-branch/gcc/testsuite/ChangeLog
    branches/gcc-9-branch/gcc/tree-cfg.c
    branches/gcc-9-branch/gcc/tree-ssa-phiopt.c
    branches/gcc-9-branch/gcc/tree-ssa-sccvn.c
    branches/gcc-9-branch/gcc/tree-vect-slp.c
Comment 11 Richard Biener 2019-08-30 11:39:51 UTC
Author: rguenth
Date: Fri Aug 30 11:39:19 2019
New Revision: 275100

URL: https://gcc.gnu.org/viewcvs?rev=275100&root=gcc&view=rev
Log:
2019-08-30  Richard Biener  <rguenther@suse.de>

	Backport from mainline
	2019-08-12  Richard Biener  <rguenther@suse.de>

	PR lto/91375
	* tree.c (free_lang_data_in_type): Do not free TYPE_BINFO dependent on
	flag_devirtualize.

	2019-07-31  Richard Biener  <rguenther@suse.de>

	PR tree-optimization/91293
	* tree-vect-slp.c (vect_build_slp_tree_2): Do not swap operands
	of reduction stmts.

	* gcc.dg/vect/pr91293-1.c: New testcase.
	* gcc.dg/vect/pr91293-2.c: Likewise.
	* gcc.dg/vect/pr91293-3.c: Likewise.

	2019-07-31  Richard Biener  <rguenther@suse.de>

	PR tree-optimization/91280
	* tree-ssa-structalias.c (get_constraint_for_component_ref):
	Decompose MEM_REF manually for offset handling.

	* g++.dg/torture/pr91280.C: New testcase.

	2019-07-19  Richard Biener  <rguenther@suse.de>

	PR tree-optimization/91200
	* tree-ssa-phiopt.c (cond_store_replacement): Check we have
	no PHI nodes in middle-bb.

	* gcc.dg/torture/pr91200.c: New testcase.

	2019-07-15  Richard Biener  <rguenther@suse.de>

	PR middle-end/91162
	* tree-cfg.c (move_block_to_fn): When releasing a virtual PHI
	node make sure to replace all uses with something valid.

	* gcc.dg/autopar/pr91162.c: New testcase.

	2019-07-11  Richard Biener  <rguenther@suse.de>

	PR middle-end/91131
	* gimplify.c (gimplify_compound_literal_expr): Force a temporary
	when the object is volatile and we have not cleared it even though
	there are no nonzero elements.

	* gcc.target/i386/pr91131.c: New testcase.

	2019-07-10  Richard Biener  <rguenther@suse.de>

	PR tree-optimization/91126
	* tree-ssa-sccvn.c (vn_reference_lookup_3): Adjust
	native encoding offset for BYTES_BIG_ENDIAN.

	* gcc.dg/torture/pr91126.c: New testcase.

Added:
    branches/gcc-8-branch/gcc/testsuite/g++.dg/torture/pr91280.C
    branches/gcc-8-branch/gcc/testsuite/gcc.dg/autopar/pr91162.c
    branches/gcc-8-branch/gcc/testsuite/gcc.dg/torture/pr91126.c
    branches/gcc-8-branch/gcc/testsuite/gcc.dg/torture/pr91200.c
    branches/gcc-8-branch/gcc/testsuite/gcc.dg/vect/pr91293-1.c
    branches/gcc-8-branch/gcc/testsuite/gcc.dg/vect/pr91293-2.c
    branches/gcc-8-branch/gcc/testsuite/gcc.dg/vect/pr91293-3.c
    branches/gcc-8-branch/gcc/testsuite/gcc.target/i386/pr91131.c
Modified:
    branches/gcc-8-branch/gcc/ChangeLog
    branches/gcc-8-branch/gcc/gimplify.c
    branches/gcc-8-branch/gcc/testsuite/ChangeLog
    branches/gcc-8-branch/gcc/tree-cfg.c
    branches/gcc-8-branch/gcc/tree-ssa-phiopt.c
    branches/gcc-8-branch/gcc/tree-ssa-sccvn.c
    branches/gcc-8-branch/gcc/tree-ssa-structalias.c
    branches/gcc-8-branch/gcc/tree-vect-slp.c
    branches/gcc-8-branch/gcc/tree.c
Comment 12 Richard Biener 2019-09-02 12:56:55 UTC
Author: rguenth
Date: Mon Sep  2 12:56:24 2019
New Revision: 275317

URL: https://gcc.gnu.org/viewcvs?rev=275317&root=gcc&view=rev
Log:
2019-09-02  Richard Biener  <rguenther@suse.de>

	Backport from mainline
	2019-07-19  Richard Biener  <rguenther@suse.de>

	PR tree-optimization/91200
	* tree-ssa-phiopt.c (cond_store_replacement): Check we have
	no PHI nodes in middle-bb.

	* gcc.dg/torture/pr91200.c: New testcase.

	2019-07-15  Richard Biener  <rguenther@suse.de>

	PR middle-end/91162
	* tree-cfg.c (move_block_to_fn): When releasing a virtual PHI
	node make sure to replace all uses with something valid.

	* gcc.dg/autopar/pr91162.c: New testcase.

	2019-07-11  Richard Biener  <rguenther@suse.de>

	PR middle-end/91131
	* gimplify.c (gimplify_compound_literal_expr): Force a temporary
	when the object is volatile and we have not cleared it even though
	there are no nonzero elements.

	* gcc.target/i386/pr91131.c: New testcase.

	2019-07-10  Richard Biener  <rguenther@suse.de>

	PR tree-optimization/91126
	* tree-ssa-sccvn.c (n_walk_cb_data::push_partial_def): Adjust
	native encoding offset for BYTES_BIG_ENDIAN.
	(vn_reference_lookup_3): Likewise.

	* gcc.dg/torture/pr91126.c: New testcase.

	2019-07-10  Richard Biener  <rguenther@suse.de>

	PR tree-optimization/91126
	* tree-ssa-sccvn.c (vn_reference_lookup_3): Adjust
	native encoding offset for BYTES_BIG_ENDIAN.

	* gcc.dg/torture/pr91126.c: New testcase.

	2019-04-29  Richard Biener  <rguenther@suse.de>

	PR tree-optimization/90278
	* tree-ssa-forwprop.c (pass_forwprop::execute): Transfer/clean
	EH on comparison simplification.

	* gcc.dg/torture/pr90278.c: New testcase.

	2019-04-11  Richard Biener  <rguenther@suse.de>

	PR tree-optimization/90020
	* tree-ssa-sccvn.c (vn_reference_may_trap): New function.
	* tree-ssa-sccvn.h (vn_reference_may_trap): Declare.
	* tree-ssa-pre.c (compute_avail): Use it to not put
	possibly trapping references after a call that might not
	return into EXP_GEN.
	* gcse.c (compute_hash_table_work): Do not elide
	marking a block containing a call if the call might not
	return.

	* gcc.dg/torture/pr90020.c: New testcase.

Added:
    branches/gcc-7-branch/gcc/testsuite/gcc.dg/autopar/pr91162.c
    branches/gcc-7-branch/gcc/testsuite/gcc.dg/torture/pr90020.c
    branches/gcc-7-branch/gcc/testsuite/gcc.dg/torture/pr90278.c
    branches/gcc-7-branch/gcc/testsuite/gcc.dg/torture/pr91126.c
    branches/gcc-7-branch/gcc/testsuite/gcc.dg/torture/pr91200.c
    branches/gcc-7-branch/gcc/testsuite/gcc.target/i386/pr91131.c
Modified:
    branches/gcc-7-branch/gcc/ChangeLog
    branches/gcc-7-branch/gcc/gcse.c
    branches/gcc-7-branch/gcc/gimplify.c
    branches/gcc-7-branch/gcc/testsuite/ChangeLog
    branches/gcc-7-branch/gcc/tree-cfg.c
    branches/gcc-7-branch/gcc/tree-ssa-forwprop.c
    branches/gcc-7-branch/gcc/tree-ssa-phiopt.c
    branches/gcc-7-branch/gcc/tree-ssa-pre.c
    branches/gcc-7-branch/gcc/tree-ssa-sccvn.c
    branches/gcc-7-branch/gcc/tree-ssa-sccvn.h
Comment 13 Richard Biener 2019-09-02 12:58:12 UTC
Fixed.