| Message ID | CAFiYyc0MrXj-rU8vdCRu4G1vNeUu-jMMz-v1nVsiKx+A0sQ=2g@mail.gmail.com |
|---|---|
| State | New |
| Headers | show |
Thanks for your feedback Richard, On Apr 25, 2012, at 16:16 , Richard Guenther wrote: > I think much better would be to simply disallow any toplevel > VIEW_CONVERT_EXPR of BLKmode, > Does that fix your problems, too? If so I prefer that. Hmm, I think that this would fix the particular testscase at hand but not quite the underlying issue I was aiming at. The basic problem I am trying to address is SRA turning VCE (<some access>) into VCE (<scalar>) where the replacement <scalar> is of a different size than <some access>. The idea of locating the check in create_access is that this is where the <access> size is computed. I'm pretty sure that excluding just on BLKmode for TYPE(<access>) doesn't catch them all. In particular, I'm pretty sure that we can get component refs of integral modes that access a smaller range of bits than what the mode conveys. It is common with packing or rep clauses in Ada. I'll see if I can come up with a case tomorrow. I'm actually also slightly confused by the processing of VCE inputs in SRA, as VCE (<composite>) and VCE (<scalar>) are not supposed to result in the same outcome for identical bit size and patterns on big endian. But I might just still be missing something here. Olivier
On Wed, Apr 25, 2012 at 11:29 PM, Olivier Hainque <hainque@adacore.com> wrote: > Thanks for your feedback Richard, > > On Apr 25, 2012, at 16:16 , Richard Guenther wrote: >> I think much better would be to simply disallow any toplevel >> VIEW_CONVERT_EXPR of BLKmode, > >> Does that fix your problems, too? If so I prefer that. > > Hmm, I think that this would fix the particular testscase at > hand but not quite the underlying issue I was aiming at. > > The basic problem I am trying to address is SRA turning > VCE (<some access>) into VCE (<scalar>) where the replacement > <scalar> is of a different size than <some access>. > > The idea of locating the check in create_access is that > this is where the <access> size is computed. > > I'm pretty sure that excluding just on BLKmode for > TYPE(<access>) doesn't catch them all. > > In particular, I'm pretty sure that we can get component > refs of integral modes that access a smaller range of bits > than what the mode conveys. It is common with packing or > rep clauses in Ada. Yeah, well - the tree verification code in tree-cfg.c is not enforcing any constraints here and the docs are not clear either. My view is that we don't want the size of the VIEW_CONVERT_EXPR differ from the size of its operand - you should use a BIT_FIELD_REF or a MEM_REF for that (yes, VIEW_CONVERT_EXPRs could be removed or treated as short-cut for a BIT_FIELD_REF that covers the whole object). > I'll see if I can come up with a case tomorrow. > > I'm actually also slightly confused by the processing of > VCE inputs in SRA, as VCE (<composite>) and VCE (<scalar>) > are not supposed to result in the same outcome for identical > bit size and patterns on big endian. But I might just still > be missing something here. Well, it's not clear to me either what the semantics of a VIEW_CONVERT_EXPR is when you apply it to memory and the result is not of the same size as the operand. Naively a VIEW_CONVERT_EXPR is *(T *)&op? Then a VIEW_CONVERT_EXPR <T, op> would be the same as MEM_REF <T, &op, (T' *)0> with the advantage that for the MEM_REF you can specify the alias set that is supposed to be in effect. Similar it would be the same as BIT_FIELD_REF <T, &op, sizeof (T) * BITS_PER_UNIT, 0>. Can you formally relate those three representations and tell me why VIEW_CONVERT_EXPR is useful (not only convenient because of less operands) to use on lvalues (thus memory, compared to registers or constants)? Thanks, Richard. > Olivier >
Hello Richard, Thanks for the constructive exchange :-) On Apr 26, 2012, at 10:48 , Richard Guenther wrote: >> In particular, I'm pretty sure that we can get component >> refs of integral modes that access a smaller range of bits >> than what the mode conveys. It is common with packing or >> rep clauses in Ada. > > Yeah, well - the tree verification code in tree-cfg.c is not enforcing > any constraints here and the docs are not clear either. My view is > that we don't want the size of the VIEW_CONVERT_EXPR differ from the > size of its operand I agree the current situation is not ideal. What you suggest corresponds to what is currently documented (difference => undefined behavior), but I'm pretty sure that the Ada compiler relies on some cases to behave in some specific manner. For tagged types IIRC. It would certainly be nice to get rid of this discrepancy at some point. The issue I was trying to address was a bit different: SRA changing the VCE argument "access" size, which seems incorrect regardless of the source destination size consistency. I can see how they are connected though and will see if I can come up with a different approach. [...] > Naively a VIEW_CONVERT_EXPR is *(T *)&op? Then a > VIEW_CONVERT_EXPR <T, op> would be the same as > MEM_REF <T, &op, (T' *)0> with the advantage that for the MEM_REF you > can specify the alias set that is supposed to be in effect. Similar > it would be the same as BIT_FIELD_REF <T, &op, sizeof (T) * BITS_PER_UNIT, 0>. > Can you formally relate those three representations and tell me why > VIEW_CONVERT_EXPR is useful (not only convenient because of less operands) > to use on lvalues (thus memory, compared to registers or constants)? I have ideas on how they are supposed to relate (corresponding to your intuitive understanding, what is documented), but MEM_REF and BIT_FIELD_REF were introduced much more recently though, so I'm pretty sure that there are details that escape me.
On Apr 30, 2012, at 16:18 , Olivier Hainque wrote: >> Can you formally relate those three representations and tell me why >> VIEW_CONVERT_EXPR is useful (not only convenient because of less operands) >> to use on lvalues (thus memory, compared to registers or constants)? > > I have ideas on how they are supposed to relate (corresponding to your > intuitive understanding, what is documented), but MEM_REF and BIT_FIELD_REF > were introduced much more recently though, so I'm pretty sure that there are > details that escape me. One area of potential difference came to mind yesterday: regarding the processing of type alignment differences. VCE to more aligned (of the same size) would make a temp copy to yield a correctly aligned object. Would MEM_REF do that as well ?
On Thu, May 3, 2012 at 10:21 AM, Olivier Hainque <hainque@adacore.com> wrote: > > On Apr 30, 2012, at 16:18 , Olivier Hainque wrote: >>> Can you formally relate those three representations and tell me why >>> VIEW_CONVERT_EXPR is useful (not only convenient because of less operands) >>> to use on lvalues (thus memory, compared to registers or constants)? >> >> I have ideas on how they are supposed to relate (corresponding to your >> intuitive understanding, what is documented), but MEM_REF and BIT_FIELD_REF >> were introduced much more recently though, so I'm pretty sure that there are >> details that escape me. > > One area of potential difference came to mind yesterday: regarding the > processing of type alignment differences. VCE to more aligned (of the > same size) would make a temp copy to yield a correctly aligned object. > > Would MEM_REF do that as well ? A temporary copy? You mean if I do VIEW_CONVERT_EXPR <double> (longlong) on x86 with -malign-double (so long long is 4 byte aligned and double is 8 byte aligned) the access guarantees only 4 byte alignment but if we "spill" that to a temporary the temporary will use double as type and thus get larger alignment of 8 bytes? MEM_REF <double> (&longlong) would assume 8-byte alignment of longlong unless you use a double type variant with 4-byte alignment. Richard.
On May 3, 2012, at 12:24 , Richard Guenther wrote: >> One area of potential difference came to mind yesterday: regarding the >> processing of type alignment differences. VCE to more aligned (of the >> same size) would make a temp copy to yield a correctly aligned object. >> >> Would MEM_REF do that as well ? > > A temporary copy? At least in some cases. > You mean if I do VIEW_CONVERT_EXPR <double> (longlong) > on x86 with -malign-double (so long long is 4 byte aligned and double is 8 byte > aligned) the access guarantees only 4 byte alignment but if we "spill" that to > a temporary the temporary will use double as type and thus get larger alignment > of 8 bytes? Probably not on x86. I am referring to this piece of the RTL expansion: expand_expr_real_1() case VIEW_CONVERT_EXPR: ... /* At this point, OP0 is in the correct mode. If the output type is such that the operand is known to be aligned, indicate that it is. Otherwise, we need only be concerned about alignment for non-BLKmode results. */ if (MEM_P (op0)) ... else if (STRICT_ALIGNMENT && mode != BLKmode && MEM_ALIGN (op0) < GET_MODE_ALIGNMENT (mode)) { tree inner_type = TREE_TYPE (treeop0); HOST_WIDE_INT temp_size = MAX (int_size_in_bytes (inner_type), (HOST_WIDE_INT) GET_MODE_SIZE (mode)); rtx new_rtx = assign_stack_temp_for_type (mode, temp_size, 0, type); rtx new_with_op0_mode = adjust_address (new_rtx, GET_MODE (op0), 0); gcc_assert (!TREE_ADDRESSABLE (exp)); if (GET_MODE (op0) == BLKmode) emit_block_move (new_with_op0_mode, op0, GEN_INT (GET_MODE_SIZE (mode)), (modifier == EXPAND_STACK_PARM ? BLOCK_OP_CALL_PARM : BLOCK_OP_NORMAL)); else emit_move_insn (new_with_op0_mode, op0); op0 = new_rtx; } Not sure how to trigger this though, and the documentation of VIEW_CONVERT_EXPR is pretty silent about it :-(
Index: gcc/tree-sra.c =================================================================== --- gcc/tree-sra.c (revision 186817) +++ gcc/tree-sra.c (working copy) @@ -1001,9 +1001,10 @@ build_access_from_expr_1 (tree expr, gim /* We need to dive through V_C_Es in order to get the size of its parameter and not the result type. Ada produces such statements. We are also - capable of handling the topmost V_C_E but not any of those buried in other - handled components. */ - if (TREE_CODE (expr) == VIEW_CONVERT_EXPR) + capable of handling the topmost V_C_E if it has a suitable mode but + not any of those buried in other handled components. */ + if (TREE_CODE (expr) == VIEW_CONVERT_EXPR + && TYPE_MODE (TREE_TYPE (expr)) != BLKmode) expr = TREE_OPERAND (expr, 0); if (contains_view_convert_expr_p (expr))