> Now, back to PR53970, where #pragma pack() is used to pack a
> struct.  With #pragma pack() no part of the type or field-decls
> have a hint that packing took place (well, their align information
> tell you), which means the vectorizers use of contains_packed_reference
> is not conservative enough, nor is expand_exprs use:
> 
>     case BIT_FIELD_REF:
>     case ARRAY_RANGE_REF:
>     normal_inner_ref:
>       {
> ...
>         if (TYPE_PACKED (TREE_TYPE (TREE_OPERAND (exp, 0)))
> 
>             || (TREE_CODE (TREE_OPERAND (exp, 1)) == FIELD_DECL
> 
>                 && DECL_PACKED (TREE_OPERAND (exp, 1))))
>           packedp = true;
> 
> I'm not sure if this flag is required for correctness - it's only
> passed to extract_bit_field - but if it is the above code does
> not work for #pragma packed structs.  I suppose what should be
> checked is (a few lines below the above test, after we expanded tem)
> 
>         if (MEM_P (op0)
>             && GET_MODE (op0) != BLKmode
>           && MEM_ALIGN (op0) < GET_MODE_ALIGNMENT (GET_MODE (op0)))
>           packedp = true;
> 
> ?  I suppose packedp was computed for STRICT_ALIGNMENT targets only.
> I'm not changing the above, but Eric, you should be able to produce a
> #pragma packed testcase that fails on a STRICT_ALIGNMENT target?

This is the -fstrict-volatile-bitfields business though, and its documentation 
explicitly refers to the packed attribute:

     If the target requires strict alignment, and honoring the field
     type would require violating this alignment, a warning is issued.
     If the field has `packed' attribute, the access is done without
     honoring the field type.  If the field doesn't have `packed'
     attribute, the access is done honoring the field type.  In both
     cases, GCC assumes that the user knows something about the target
     hardware that it is unaware of.

so I'm a little reluctant to touch that.  But, yes, generally speaking, testing 
TYPE_PACKED or DECL_PACKED to drive code generation is wrong.

> Oh, and this does not yet fix PR53970 - but I hope that I can
> remove contains_packed_reference ;)

Right, it should definitely go away.

-- 
Eric Botcazou

Reply via email to