Richard Biener <rguent...@suse.de> writes:
> On Thu, 3 Feb 2022, Richard Sandiford wrote:
>
>> Richard Biener <rguent...@suse.de> writes:
>> > On Wed, 2 Feb 2022, Michael Matz wrote:
>> >
>> >> Hello,
>> >>
>> >> On Wed, 2 Feb 2022, Richard Biener via Gcc-patches wrote:
>> >>
>> >> > This adds a flag to CONSTRUCTOR nodes indicating that for clobbers this
>> >> > marks the end-of-life of storage as opposed to just ending the lifetime
>> >> > of the object that occupied it. The dangling pointer diagnostics uses
>> >> > CLOBBERs but is confused by those emitted by the C++ frontend for
>> >> > example which emits them for the second purpose at the start of CTORs.
>> >> > The issue is also appearant for aarch64 in PR104092.
>> >> >
>> >> > Distinguishing the two cases is also necessary for the PR90348 fix.
>> >>
>> >> (Just FWIW: I agree with the plan to have different types of CLOBBERs, in
>> >> particular those for marking births)
>> >>
>> >> A style nit:
>> >>
>> >> > tree clobber = build_clobber (TREE_TYPE (t));
>> >> > + CLOBBER_MARKS_EOL (clobber) = 1;
>> >>
>> >> I think it really were better if build_clobber() gained an additional
>> >> argument (non-optional!) that sets the type of it.
>> >
>> > It indeed did occur to me as well, so as we're two now I tried to see
>> > how it looks like. It does like the following (didn't bother to
>> > replace all build_clobber calls but defaulted the parameter - there
>> > are too many). With CLOBBER_BIRTH added for the fix for PR90348
>> > the enum would be constructed like
>> >
>> > #define CLOBBER_KIND(NODE) \
>> > ((clobber_kind) (CONSTRUCTOR_CHECK (NODE)->base.private_flag << 1
>> > | CONSTRUCTOR_CHECK (NODE)->base.protected_flag))
>> >
>> > or so (maybe different dependent on bitfield endianess to make
>> > extracting the two adjacent bits cheap). That would leave us space
>> > for a fourth state.
>> >
>> > So do you think that makes it nicer? What do others think?
>>
>> This way looks cleaner to me FWIW.
>>
>> Which arm of tree_base::u do constructors use? If we need a 2-bit
>> enum then maybe we can carve one out from there.
>
> constructors are tcc_exceptional but since they are used where
> tree operands reside they need to be compatible to EXPR_P and
> thus various things like TREE_SIDE_EFFECTS need to work.
>
> Which means, we can't.
But TREE_SIDE_EFFECTS is part of the main set of flags. For tree_base::u
we have:
union {
/* The bits in the following structure should only be used with
accessor macros that constrain inputs with tree checking. */
struct {
unsigned lang_flag_0 : 1;
unsigned lang_flag_1 : 1;
unsigned lang_flag_2 : 1;
unsigned lang_flag_3 : 1;
unsigned lang_flag_4 : 1;
unsigned lang_flag_5 : 1;
unsigned lang_flag_6 : 1;
unsigned saturating_flag : 1;
unsigned unsigned_flag : 1;
unsigned packed_flag : 1;
unsigned user_align : 1;
unsigned nameless_flag : 1;
unsigned atomic_flag : 1;
unsigned unavailable_flag : 1;
unsigned spare0 : 2;
unsigned spare1 : 8;
/* This field is only used with TREE_TYPE nodes; the only reason it is
present in tree_base instead of tree_type is to save space. The size
of the field must be large enough to hold addr_space_t values. */
unsigned address_space : 8;
} bits;
/* The following fields are present in tree_base to save space. The
nodes using them do not require any of the flags above and so can
make better use of the 4-byte sized word. */
/* The number of HOST_WIDE_INTs in an INTEGER_CST. */
struct {
/* The number of HOST_WIDE_INTs if the INTEGER_CST is accessed in
its native precision. */
unsigned char unextended;
/* The number of HOST_WIDE_INTs if the INTEGER_CST is extended to
wider precisions based on its TYPE_SIGN. */
unsigned char extended;
/* The number of HOST_WIDE_INTs if the INTEGER_CST is accessed in
offset_int precision, with smaller integers being extended
according to their TYPE_SIGN. This is equal to one of the two
fields above but is cached for speed. */
unsigned char offset;
} int_length;
/* VEC length. This field is only used with TREE_VEC. */
int length;
/* This field is only used with VECTOR_CST. */
struct {
/* The value of VECTOR_CST_LOG2_NPATTERNS. */
unsigned int log2_npatterns : 8;
/* The value of VECTOR_CST_NELTS_PER_PATTERN. */
unsigned int nelts_per_pattern : 8;
/* For future expansion. */
unsigned int unused : 16;
} vector_cst;
/* SSA version number. This field is only used with SSA_NAME. */
unsigned int version;
/* CHREC_VARIABLE. This field is only used with POLYNOMIAL_CHREC. */
unsigned int chrec_var;
/* Internal function code. */
enum internal_fn ifn;
/* OMP_ATOMIC* memory order. */
enum omp_memory_order omp_atomic_memory_order;
/* The following two fields are used for MEM_REF and TARGET_MEM_REF
expression trees and specify known data non-dependences. For
two memory references in a function they are known to not
alias if dependence_info.clique are equal and dependence_info.base
are distinct. Clique number zero means there is no information,
clique number one is populated from function global information
and thus needs no remapping on transforms like loop unrolling. */
struct {
unsigned short clique;
unsigned short base;
} dependence_info;
} GTY((skip(""))) u;
so there's spare room even if constructors use the general “bits” arm.
Thanks,
Richard
