On Wed, Dec 15, 2021 at 11:26:48PM +0530, Siddhesh Poyarekar wrote:
> > This makes me a little bit worried. Do you compute the wholesize SSA_NAME
> > at runtime always, or only when it is really needed and known not to always
> > be equal to the size?
> > I mean, e.g. for the cases where there is just const char *p = malloc
> > (size);
> > and the pointer is never increased size == wholesize. For __bos it will
> > just be 2 different INTEGER_CSTs, but if it would at runtime mean we compute
> > something twice and hope we eventually find out during later passes
> > it is the same, it would be bad.
>
> I'm emitting both size and wholesize all the time; wholesize only really
> gets used in size_for_offset and otherwise should get DCE'd. Right now for
> __bos (and constant sizes) wholesize is unused if it is the same as size.
>
> FOR GIMPLE_CALL, GIMPLE_NOP, etc. I return the same tree for size and
> wholesize; maybe a trivial pointer comparison (sz != wholesize) ought to get
> rid of most of the uses in size_for_offset.
Perhaps DCE can handle well when you compute something (wholesize) that isn't
really
needed and VN/CSE the case where size and wholesize is equal. I think it
would be worth looking at a few testcases.
> > > + {
> > > + edge e = gimple_phi_arg_edge (obj_phi, i);
> > > +
> > > + /* Put the size definition before the last statement of the source
> > > + block of the PHI edge. This ensures that any branches at the end
> > > + of the source block remain the last statement. We are OK even if
> > > + the last statement is the definition of the object since it will
> > > + succeed any definitions that contribute to its size and the size
> > > + expression will succeed them too. */
> > > + gimple_stmt_iterator gsi = gsi_last_bb (e->src);
> > > + gsi_insert_seq_before (&gsi, seq, GSI_CONTINUE_LINKING);
> >
> > This looks problematic. The last stmt in the bb might not exist at all,
>
> Wouldn't the bb minimally have to contain the definition of the object whose
> size we computed? e.g. for PHI [a(2), b(3)], wouldn't bb 2 at least have a
> statement with the definition of a?
It can e.g. contain just a PHI.
> Or wait, there could be situations where the definition is in a different
> block, e.g. bb 1, which has a single edge going on to bb 2?
> I suppose __bos-like behaviour could be a good compromise, i.e. insert a
> MAX_EXPR (or MIN_EXPR) if we can't find a suitable location to insert on
> edge.
MAX_EXPR or MIN_EXPR? I'd have expect the __bos constant in there.
But I must say I'm right now unsure what kind of PHIs one can have on bbs
reachable from both ab/eh edges and normal edges if we can have such bbs at
all. I guess looking at some sigjmp/longjmp or non-local or computed goto
testcases might show something, perhaps I'll have a look tomorrow.
I'm sure we can have vop PHI.
Jakub
