Hi,

> On Tue, 8 Oct 2013 22:50:21, Eric Botcazou wrote:
>>
>>> I agree, that assigning a non-BLKmode to structures with zero-sized arrays
>>> should be considered a bug.
>>
>> Fine, then let's apply Martin's patch, on mainline at least.
>>
>
> That would definitely be a good move. Maybe someone should approve it?
>
>> But this testcase is invalid on STRICT_ALIGNMENT platforms: xx is pointer to 
>> a
>> type with 4-byte alignment so its value must be a multiple of 4.
>
> Then you probably win. But I still have some doubts.
>
> I had to use this silly alignment/pack(4) to circumvent this statement
> in compute_record_mode:
>
>   /* If structure's known alignment is less than what the scalar
>      mode would need, and it matters, then stick with BLKmode.  */
>   if (TYPE_MODE (type) != BLKmode
>       && STRICT_ALIGNMENT
>       && ! (TYPE_ALIGN (type)>= BIGGEST_ALIGNMENT
>             || TYPE_ALIGN (type)>= GET_MODE_ALIGNMENT (TYPE_MODE (type))))
>     {
>       /* If this is the only reason this type is BLKmode, then
>          don't force containing types to be BLKmode.  */
>       TYPE_NO_FORCE_BLK (type) = 1;
>       SET_TYPE_MODE (type, BLKmode);
>     }
>
> But there are at least two targets where STRICT_ALIGNMENT = 0
> and SLOW_UNALIGNED_ACCESS != 0: rs6000 and alpha.
>
> This example with a byte-aligned structure will on one of these targets
> likely execute this code path in  expand_expr_real_1/case MEM_REF:
>
>             else if (SLOW_UNALIGNED_ACCESS (mode, align))
>               temp = extract_bit_field (temp, GET_MODE_BITSIZE (mode),
>                                         0, TYPE_UNSIGNED (TREE_TYPE (exp)),
>                                         (modifier == EXPAND_STACK_PARM
>                                          ? NULL_RTX : target),
>                                         mode, mode);
>
> This looks wrong, but unfortunately I cannot test on these targets...
>

Hmm, well,

the condition that would be necessary to execute that code path
would be

STRICT_ALIGNMENT = 0
and SLOW_UNALIGNED_ACCESS != 0 for any integer mode.

The only target that is close to hit this "bug" is rs6000:

#define STRICT_ALIGNMENT 0

#define SLOW_UNALIGNED_ACCESS(MODE, ALIGN)                              \
  (STRICT_ALIGNMENT                                                     \
   || (((MODE) == SFmode || (MODE) == DFmode || (MODE) == TFmode        \
        || (MODE) == SDmode || (MODE) == DDmode || (MODE) == TDmode)    \
       && (ALIGN) < 32)                                                 \
   || (VECTOR_MODE_P ((MODE)) && (((int)(ALIGN)) < VECTOR_ALIGN (MODE))))

but, luckily this is 0 for all integer modes.

So I am now convinced, there won't be any valid example with unions that
executes this code path.

Therefore I updated Martin's previous patch, to meet Eric's request:
That is to only handle zero-sized arrays at the end of the structure.

Boot-strapped and regression-tested on x86_64-linux-gnu.

Ok for trunk?

Regards
Bernd.                                    
2013-10-22  Martin Jambor  <mjam...@suse.cz>
            Bernd Edlinger  <bernd.edlin...@hotmail.de>

        PR middle-end/57748
        * stor-layout.c (compute_record_mode): Treat trailing zero-sized array
        fields like incomplete types.

testsuite:
2013-10-22  Bernd Edlinger  <bernd.edlin...@hotmail.de>

        PR middle-end/57748
        * gcc.dg/torture/pr57748-3.c: New test.

Attachment: patch-pr57748-2.diff
Description: Binary data

Reply via email to