Re: [patch] Fix wrong code with VCE to bit-field type at -O

[Martin Jambor <mjambor at suse dot cz>]

Hi,

On Tue, Feb 11, 2014 at 02:17:04PM +0100, Richard Biener wrote:
> Hmm.  The intent was of course to only allow truly no-op converts via
> VIEW_CONVERT_EXPR - that is, the size of the operand type and the
> result type should be the same.  So, isn't SRA doing it wrong when
> creating the VIEW_CONVERT_EXPR of a 3-byte type to a 4-byte type?
> 

Even though I cannot recall the details, I remember I had to make SRA
able to accept such size mismatches in between V_C_E operands and
results in its input, to be able to work on Ada FE generated IL.  I
believe that is still the case and so we would have these mismatches
even if we changed SRA not to create them.

The reason why SRA creates such a thing is that
get_ref_base_and_extent returns size 24 when run on the following, an
expression of type with type size 32:

 <component_ref 0x7ffc45212240
    type <integer_type 0x7ffc451fe930 opt31__time_t___XDLU_0__11059199
        type <integer_type 0x7ffc451dc5e8 opt31__Tunsigned_24B sizes-gimplified public unsigned SI
            size <integer_cst 0x7ffc450573a0 constant 32>
            unit size <integer_cst 0x7ffc450573c0 constant 4>
            align 32 symtab 0 alias set -1 canonical type 0x7ffc451dc5e8 precision 32 min <integer_cst 0x7ffc451f85a0 0> max <integer_cst 0x7ffc450a1cc0 4294967295> context <function_decl 0x7ffc451fb400 opt31> RM size <integer_cst 0x7ffc450573a0 32>
            chain <type_decl 0x7ffc45084170 opt31__Tunsigned_24B>>
        sizes-gimplified public unsigned SI
        size <integer_cst 0x7ffc450573a0 constant 32>
        unit size <integer_cst 0x7ffc450573c0 constant 4>
        align 32 symtab 0 alias set -1 canonical type 0x7ffc451fe930 precision 24 min <integer_cst 0x7ffc451f8ec0 0> max <integer_cst 0x7ffc451f8e60 16777215> RM size <integer_cst 0x7ffc45057800 24> RM min <integer_cst 0x7ffc451f85a0 0> RM max <integer_cst 0x7ffc451f8800 11059199>>
   
    arg 0 <component_ref 0x7ffc45212270
        type <record_type 0x7ffc451fe888 opt31__rec1 sizes-gimplified packed BLK
            size <integer_cst 0x7ffc45057800 constant 24>
            unit size <integer_cst 0x7ffc45083100 constant 3>
            align 8 symtab 0 alias set -1 canonical type 0x7ffc451fe888 fields <field_decl 0x7ffc4507de40 f> context <function_decl 0x7ffc451fb400 opt31>
            Ada size <integer_cst 0x7ffc45057800 constant 24>
            chain <type_decl 0x7ffc45202000 opt31__rec1>>
       
        arg 0 <var_decl 0x7ffc45204260 my_rec2 type <record_type 0x7ffc451fed20 opt31__rec2>
            BLK file opt31.adb line 31 col 5
            size <integer_cst 0x7ffc45203180 constant 160>
            unit size <integer_cst 0x7ffc452031a0 constant 20>
            align 32 context <function_decl 0x7ffc451fb900 opt31__decode>>
        arg 1 <field_decl 0x7ffc45204098 r1 type <record_type 0x7ffc451fe888 opt31__rec1>
            BLK file opt31.adb line 25 col 5
            size <integer_cst 0x7ffc45057800 constant 24>
            unit size <integer_cst 0x7ffc45083100 constant 3>
            align 8 offset_align 128
            offset <integer_cst 0x7ffc450570c0 constant 16>
            bit offset <integer_cst 0x7ffc450570e0 constant 0> context <record_type 0x7ffc451fed20 opt31__rec2>>>
    arg 1 <field_decl 0x7ffc4507de40 f
        type <integer_type 0x7ffc451fe930 opt31__time_t___XDLU_0__11059199 type <integer_type 0x7ffc451dc5e8 opt31__Tunsigned_24B>
            sizes-gimplified public unsigned SI
            size <integer_cst 0x7ffc450573a0 constant 32>
            unit size <integer_cst 0x7ffc450573c0 constant 4>
            align 32 symtab 0 alias set -1 canonical type 0x7ffc451fe930 precision 24 min <integer_cst 0x7ffc451f8ec0 0> max <integer_cst 0x7ffc451f8e60 16777215> RM size <integer_cst 0x7ffc45057800 24> RM min <integer_cst 0x7ffc451f85a0 0> RM max <integer_cst 0x7ffc451f8800 11059199>>
        unsigned external packed bit-field nonaddressable SI file opt31.adb line 16 col 5
        size <integer_cst 0x7ffc45057800 constant 24>
        unit size <integer_cst 0x7ffc45083100 constant 3>
        align 1 offset_align 128
        offset <integer_cst 0x7ffc45057060 constant 0>
        bit offset <integer_cst 0x7ffc450570e0 constant 0>
        bit_field_type <integer_type 0x7ffc451fe930 opt31__time_t___XDLU_0__11059199 type <integer_type 0x7ffc451dc5e8 opt31__Tunsigned_24B>
            sizes-gimplified public unsigned SI
            size <integer_cst 0x7ffc450573a0 constant 32>
            unit size <integer_cst 0x7ffc450573c0 constant 4>
            align 32 symtab 0 alias set -1 canonical type 0x7ffc451fe930 precision 24 min <integer_cst 0x7ffc451f8ec0 0> max <integer_cst 0x7ffc451f8e60 16777215> RM size <integer_cst 0x7ffc45057800 24> RM min <integer_cst 0x7ffc451f85a0 0> RM max <integer_cst 0x7ffc451f8800 11059199>> context <record_type 0x7ffc451fe888 opt31__rec1>>>

This type wins over an access to the same memory with only 24-bit
large type from another statement and when that statement is modified,
ensuing type mismatch is "fixed" by creating a VIEW_CONVERT_EXPR with
the mismatch.

I am not sure how to deal with this, given that we have mismatched
V_C_Es anyway, I'm inclined not to care and let the expander deal with
it.  But at the same I understand that it is ugly and will certainly
cause somebody more headache in the future.  I suppose that not
scalarizing here might hurt performance and would be frowned upon at
the very least.  If the fields bigger than the record approach is the
standard way of doing this, perhaps SRA can detect such cases and
produce these strange COMPONENT_REFs instead, but is it so?

Martin