This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
[PATCH][GCC7] Remove scaling of COMPONENT_REF/ARRAY_REF ops 2/3
- From: Richard Biener <rguenther at suse dot de>
- To: gcc-patches at gcc dot gnu dot org
- Date: Thu, 18 Feb 2016 15:30:36 +0100 (CET)
- Subject: [PATCH][GCC7] Remove scaling of COMPONENT_REF/ARRAY_REF ops 2/3
- Authentication-results: sourceware.org; auth=none
The following experiment resulted from looking at making
array_ref_low_bound and array_ref_element_size non-mutating. Again
I wondered why we do this strange scaling by offset/element alignment.
This exposes sth to GIMPLE (the division) that is in the end not
necessary while costing extra tree building/folding each time
array_ref_low_bound or array_ref_element_size are called (for
variable size cases, of course). And those are called a lot
via get_inner_reference and get_ref_base_and_extent.
So - the following patch gets rid of that scaling. For a "simple"
C testcase
void bar (void *);
void foo (int n)
{
struct S { struct R { int b[n]; } a[2]; int k; } s;
s.k = 1;
s.a[1].b[7] = 3;
bar (&s);
}
the difference in .gimple (and also pre expand at -O1) is:
@@ -95,10 +93,8 @@
D.1782 = D.1770 * 8;
D.1788 = D.1782 + 4;
s.1 = __builtin_alloca_with_align (D.1788, 32);
- D.1790 = D.1782 /[ex] 4;
- s.1->k{off: D.1790 * 4} = 1;
- D.1791 = D.1773 /[ex] 4;
- s.1->a[1]{lb: 0 sz: D.1791 * 4}.b[7] = 3;
+ s.1->k{off: D.1782} = 1;
+ s.1->a[1]{lb: 0 sz: D.1773}.b[7] = 3;
s.2 = s.1;
bar (s.2);
and the generated initial RTL is much smaller:
-;; s.1_10->k{off: _11 * 4} = 1;
+;; s.1_10->k{off: _7} = 1;
-(insn 21 20 22 (parallel [
- (set (reg:DI 108)
- (lshiftrt:DI (reg:DI 90 [ _7 ])
- (const_int 2 [0x2])))
- (clobber (reg:CC 17 flags))
- ]) t.c:5 -1
- (expr_list:REG_EQUAL (udiv:DI (reg:DI 90 [ _7 ])
- (const_int 4 [0x4]))
- (nil)))
-
-(insn 22 21 0 (set (mem/c:SI (plus:DI (mult:DI (reg:DI 108)
- (const_int 4 [0x4]))
- (reg/f:DI 92 [ s.1 ])) [3 s.1_10->k{off: _11 * 4}+0 S4
A32])
+(insn 20 19 0 (set (mem/c:SI (plus:DI (reg/f:DI 91 [ s.1 ])
+ (reg:DI 89 [ _7 ])) [3 s.1_10->k{off: _7}+0 S4 A32])
(const_int 1 [0x1])) t.c:5 -1
(nil))
...
-;; s.1_10->a[1]{lb: 0 sz: _13 * 4}.b[7] = 3;
+Applying pattern match.pd:83, generic-match.c:9034
-(insn 23 22 24 (parallel [
- (set (reg:DI 109)
- (ashift:DI (reg:DI 88 [ _5 ])
- (const_int 2 [0x2])))
- (clobber (reg:CC 17 flags))
- ]) t.c:4 -1
- (nil))
-
-(insn 24 23 25 (parallel [
- (set (reg:DI 110)
- (lshiftrt:DI (reg:DI 109)
- (const_int 2 [0x2])))
- (clobber (reg:CC 17 flags))
- ]) t.c:6 -1
- (expr_list:REG_EQUAL (udiv:DI (reg:DI 109)
- (const_int 4 [0x4]))
- (nil)))
+;; s.1_10->a[1]{lb: 0 sz: _6}.b[7] = 3;
-(insn 25 24 26 (parallel [
- (set (reg:DI 111)
- (plus:DI (reg:DI 110)
- (const_int 7 [0x7])))
- (clobber (reg:CC 17 flags))
- ]) t.c:6 -1
- (nil))
-
-(insn 26 25 0 (set (mem:SI (plus:DI (mult:DI (reg:DI 111)
- (const_int 4 [0x4]))
- (reg/f:DI 92 [ s.1 ])) [3 s.1_10->a[1]{lb: 0 sz: _13 *
4}.b+28 S4 A32])
+(insn 21 20 0 (set (mem:SI (plus:DI (plus:DI (mult:DI (reg:DI 87 [ _5 ])
+ (const_int 4 [0x4]))
+ (reg/f:DI 91 [ s.1 ]))
+ (const_int 28 [0x1c])) [3 s.1_10->a[1]{lb: 0 sz: _6}.b+28
S4 A32])
(const_int 3 [0x3])) t.c:6 -1
(nil))
assembler difference is
@@ -11,12 +11,11 @@
movq %rsp, %rbp
.cfi_def_cfa_register 6
movslq %edi, %rax
- leaq 0(,%rax,8), %rcx
- leaq 22(%rcx), %rdx
+ leaq 22(,%rax,8), %rdx
andq $-16, %rdx
subq %rdx, %rsp
movq %rsp, %rdi
- movl $1, (%rsp,%rcx)
+ movl $1, (%rsp,%rax,8)
movl $3, 28(%rsp,%rax,4)
call bar
leave
which may be a mixed bag - I suppose the testcase is too simple to
evaluate code changes.
I tried to go back in time and finding relevant discussions about this
but didn't find anything as the whole (very large) discussion ended
up about increasing the size of the COMPONENT_REF / ARRAY_REF trees ....
So - I hope somebody from Adacore can evaluate this patch code-generation
wise.
Bootstrapped and tested on x86_64-unknown-linux-gnu. Queued for GCC 7
if I hear nothing negative from the Ada folks.
Back to making array_ref_element_size/array_ref_low_bound non-mutating
(for the PR69553 fix). Bah, which doesn't work - substituting
placeholders during gimplification only - as PLACEHOLDER_EXPRs
leak into the IL via folding (calling get_inner_reference for example)
before gimplification. Even the ADA FE itself does this.
Thanks,
Richard.
2016-02-18 Richard Biener <rguenther@suse.de>
* tree.def (COMPONENT_REF): Adjust docs for operand 2.
(ARRAY_REF): Adjust docs for operand 3.
* gimplify.c (gimplify_compound_lval): Remove scaling of
COMPONENT_REF and ARRAY_REF operands.
* tree.c (array_ref_element_size): Remove scaling of operand 3.
(component_ref_field_offset): Remove scaling of operand 2.
* tree-ssa-pre.c (create_component_ref_by_pieces_1): Remove
kludge for ARRAY_REF operand 3 scaling issue.
Index: gcc/tree.def
===================================================================
*** gcc/tree.def (revision 233498)
--- gcc/tree.def (working copy)
*************** DEFTREECODE (TRANSLATION_UNIT_DECL, "tra
*** 422,429 ****
/* Value is structure or union component.
Operand 0 is the structure or union (an expression).
Operand 1 is the field (a node of type FIELD_DECL).
! Operand 2, if present, is the value of DECL_FIELD_OFFSET, measured
! in units of DECL_OFFSET_ALIGN / BITS_PER_UNIT. */
DEFTREECODE (COMPONENT_REF, "component_ref", tcc_reference, 3)
/* Reference to a group of bits within an object. Similar to COMPONENT_REF
--- 422,428 ----
/* Value is structure or union component.
Operand 0 is the structure or union (an expression).
Operand 1 is the field (a node of type FIELD_DECL).
! Operand 2, if present, is the value of DECL_FIELD_OFFSET. */
DEFTREECODE (COMPONENT_REF, "component_ref", tcc_reference, 3)
/* Reference to a group of bits within an object. Similar to COMPONENT_REF
*************** DEFTREECODE (BIT_FIELD_REF, "bit_field_r
*** 439,446 ****
/* Array indexing.
Operand 0 is the array; operand 1 is a (single) array index.
Operand 2, if present, is a copy of TYPE_MIN_VALUE of the index.
! Operand 3, if present, is the element size, measured in units of
! the alignment of the element type. */
DEFTREECODE (ARRAY_REF, "array_ref", tcc_reference, 4)
/* Likewise, except that the result is a range ("slice") of the array. The
--- 438,444 ----
/* Array indexing.
Operand 0 is the array; operand 1 is a (single) array index.
Operand 2, if present, is a copy of TYPE_MIN_VALUE of the index.
! Operand 3, if present, is the element size as in TYPE_SIZE_UNIT. */
DEFTREECODE (ARRAY_REF, "array_ref", tcc_reference, 4)
/* Likewise, except that the result is a range ("slice") of the array. The
Index: gcc/gimplify.c
===================================================================
*** gcc/gimplify.c (revision 233498)
--- gcc/gimplify.c (working copy)
*************** gimplify_compound_lval (tree *expr_p, gi
*** 2040,2057 ****
if (TREE_OPERAND (t, 3) == NULL_TREE)
{
! tree elmt_type = TREE_TYPE (TREE_TYPE (TREE_OPERAND (t, 0)));
! tree elmt_size = unshare_expr (array_ref_element_size (t));
! tree factor = size_int (TYPE_ALIGN_UNIT (elmt_type));
!
! /* Divide the element size by the alignment of the element
! type (above). */
! elmt_size
! = size_binop_loc (loc, EXACT_DIV_EXPR, elmt_size, factor);
if (!is_gimple_min_invariant (elmt_size))
{
! TREE_OPERAND (t, 3) = elmt_size;
tret = gimplify_expr (&TREE_OPERAND (t, 3), pre_p,
post_p, is_gimple_reg,
fb_rvalue);
--- 2040,2050 ----
if (TREE_OPERAND (t, 3) == NULL_TREE)
{
! tree elmt_size = array_ref_element_size (t);
if (!is_gimple_min_invariant (elmt_size))
{
! TREE_OPERAND (t, 3) = unshare_expr (elmt_size);
tret = gimplify_expr (&TREE_OPERAND (t, 3), pre_p,
post_p, is_gimple_reg,
fb_rvalue);
*************** gimplify_compound_lval (tree *expr_p, gi
*** 2070,2086 ****
/* Set the field offset into T and gimplify it. */
if (TREE_OPERAND (t, 2) == NULL_TREE)
{
! tree offset = unshare_expr (component_ref_field_offset (t));
! tree field = TREE_OPERAND (t, 1);
! tree factor
! = size_int (DECL_OFFSET_ALIGN (field) / BITS_PER_UNIT);
!
! /* Divide the offset by its alignment. */
! offset = size_binop_loc (loc, EXACT_DIV_EXPR, offset, factor);
if (!is_gimple_min_invariant (offset))
{
! TREE_OPERAND (t, 2) = offset;
tret = gimplify_expr (&TREE_OPERAND (t, 2), pre_p,
post_p, is_gimple_reg,
fb_rvalue);
--- 2063,2073 ----
/* Set the field offset into T and gimplify it. */
if (TREE_OPERAND (t, 2) == NULL_TREE)
{
! tree offset = component_ref_field_offset (t);
if (!is_gimple_min_invariant (offset))
{
! TREE_OPERAND (t, 2) = unshare_expr (offset);
tret = gimplify_expr (&TREE_OPERAND (t, 2), pre_p,
post_p, is_gimple_reg,
fb_rvalue);
Index: gcc/tree.c
===================================================================
*** gcc/tree.c (revision 233498)
--- gcc/tree.c (working copy)
*************** get_base_address (tree t)
*** 12862,12882 ****
tree
array_ref_element_size (tree exp)
{
! tree aligned_size = TREE_OPERAND (exp, 3);
tree elmt_type = TREE_TYPE (TREE_TYPE (TREE_OPERAND (exp, 0)));
- location_t loc = EXPR_LOCATION (exp);
! /* If a size was specified in the ARRAY_REF, it's the size measured
! in alignment units of the element type. So multiply by that value. */
! if (aligned_size)
! {
! /* ??? tree_ssa_useless_type_conversion will eliminate casts to
! sizetype from another type of the same width and signedness. */
! if (TREE_TYPE (aligned_size) != sizetype)
! aligned_size = fold_convert_loc (loc, sizetype, aligned_size);
! return size_binop_loc (loc, MULT_EXPR, aligned_size,
! size_int (TYPE_ALIGN_UNIT (elmt_type)));
! }
/* Otherwise, take the size from that of the element type. Substitute
any PLACEHOLDER_EXPR that we have. */
--- 12865,12876 ----
tree
array_ref_element_size (tree exp)
{
! tree size = TREE_OPERAND (exp, 3);
tree elmt_type = TREE_TYPE (TREE_TYPE (TREE_OPERAND (exp, 0)));
! /* If a size was specified in the ARRAY_REF, return it. */
! if (size)
! return size;
/* Otherwise, take the size from that of the element type. Substitute
any PLACEHOLDER_EXPR that we have. */
*************** array_at_struct_end_p (tree ref)
*** 12965,12987 ****
tree
component_ref_field_offset (tree exp)
{
- tree aligned_offset = TREE_OPERAND (exp, 2);
tree field = TREE_OPERAND (exp, 1);
! location_t loc = EXPR_LOCATION (exp);
! /* If an offset was specified in the COMPONENT_REF, it's the offset measured
! in units of DECL_OFFSET_ALIGN / BITS_PER_UNIT. So multiply by that
! value. */
! if (aligned_offset)
! {
! /* ??? tree_ssa_useless_type_conversion will eliminate casts to
! sizetype from another type of the same width and signedness. */
! if (TREE_TYPE (aligned_offset) != sizetype)
! aligned_offset = fold_convert_loc (loc, sizetype, aligned_offset);
! return size_binop_loc (loc, MULT_EXPR, aligned_offset,
! size_int (DECL_OFFSET_ALIGN (field)
! / BITS_PER_UNIT));
! }
/* Otherwise, take the offset from that of the field. Substitute
any PLACEHOLDER_EXPR that we have. */
--- 12959,12970 ----
tree
component_ref_field_offset (tree exp)
{
tree field = TREE_OPERAND (exp, 1);
! tree offset = TREE_OPERAND (exp, 2);
! /* If an offset was specified in the COMPONENT_REF, return it. */
! if (offset)
! return offset;
/* Otherwise, take the offset from that of the field. Substitute
any PLACEHOLDER_EXPR that we have. */
Index: gcc/tree-ssa-pre.c
===================================================================
*** gcc/tree-ssa-pre.c (revision 233516)
--- gcc/tree-ssa-pre.c (working copy)
*************** create_component_ref_by_pieces_1 (basic_
*** 2592,2607 ****
if (genop3)
{
tree elmt_type = TREE_TYPE (TREE_TYPE (genop0));
! /* We can't always put a size in units of the element alignment
! here as the element alignment may be not visible. See
! PR43783. Simply drop the element size for constant
! sizes. */
if (tree_int_cst_equal (genop3, TYPE_SIZE_UNIT (elmt_type)))
genop3 = NULL_TREE;
else
{
- genop3 = size_binop (EXACT_DIV_EXPR, genop3,
- size_int (TYPE_ALIGN_UNIT (elmt_type)));
genop3 = find_or_generate_expression (block, genop3, stmts);
if (!genop3)
return NULL_TREE;
--- 2592,2602 ----
if (genop3)
{
tree elmt_type = TREE_TYPE (TREE_TYPE (genop0));
! /* Drop the element size for constant sizes. */
if (tree_int_cst_equal (genop3, TYPE_SIZE_UNIT (elmt_type)))
genop3 = NULL_TREE;
else
{
genop3 = find_or_generate_expression (block, genop3, stmts);
if (!genop3)
return NULL_TREE;