On 1/5/21 5:38 AM, Richard Biener wrote:
On Mon, Jan 4, 2021 at 9:53 PM Martin Sebor <mse...@gmail.com> wrote:
On 1/4/21 12:23 PM, Jeff Law wrote:
On 1/4/21 12:19 PM, Jakub Jelinek wrote:
On Mon, Jan 04, 2021 at 12:14:15PM -0700, Jeff Law via Gcc-patches wrote:
Doing the STRING_CST is certainly less fragile since the SSA names
created at gimplification time could even be ggc_freed when no longer
used in the IL.
Obviously we can't use SSA_NAMEs as they're specific to each function as
they get compiled. But what's not as clear to me is why we can't use a
SAVE_EXPR of the original expression that indicates the size of the
parameter.
The gimplifier is destructive, so if the expressions are partly (e.g. in
those SAVE_EXPRs) shared with what is in the actual IL, we lose.
And if they aren't shared and there are side-effects, if we tried to
gimplify them again we'd get the side-effects duplicated.
So it all depends on what the code wants to handle, if e.g. just values of
parameters with simple arithmetics on those and punt on everything else,
then it is doable, but generally it is not.
I explained what the code handles and when in the pipeline in
the discussion of the previous patch:
https://gcc.gnu.org/pipermail/gcc-patches/2020-November/559770.html
I would expect the expressions to be values of parameters (or objects in
static storage) and simple arithemetic on them. If there's other cases,
punting seems appropriate.
Martin -- are there nontrivial expressions we need to be worried about here?
At the moment the middle warnings only consider parameters, like
the N in
void f (int N, int[N]);
void g (void)
{
int a[3];
f (sizeof a, a); // warning
I wonder how this can work reliably without heavy-weight
"parsing" of the attribute? That is, how do you relate
the passed 24 constant to the N in int[N]?
There is some parsing involved but it's only slightly more complex
than in attribute fn spec. Just a string scan followed by constant
time lookup for each pointer argument.
The N is associated with int[N] via N's position in the argument
list and encoded as $N in the string. The attribute for the decl
above is "1[$],$0". The 1 is the VLA argument position, each
dollar sign in the brackets is one VLA bound (numbers are constant
bounds), and the $0 is the VLA bound argument.
(There is some redundancy here since all but the most significant
array (or VLA) bound are also encoded in the type of the argument.)
The front end redeclaration warnings consider all expressions,
including
int f (void);
void g (int[f () + 1]);
void g (int[f () + 2]); // warning
For redeclaration warning the attribute isn't needed since you
have both decls and can compare sizes directly?
The attribute is used here as well. It's attached to the first
decl irrespective of the form of the VLA, then created for
the second decl and the two are compared. Mismatches are then
diagnosed and dropped from the second attribute. The result
is merged with the first and added to the decl.
Martin
The patch turns these complex bounds into strings that the front
end compares instead. After the front end is done the strings
don't serve any purpose (and I don't think ever will) and could
be removed. I looked for a way to do it but couldn't find one
other than the free_lang_data pass in tree.c that Richard had
initially said wasn't the right place. Sounds like he's
reconsidered but at this point, given that VLA parameters are
used only infraquently, and VLAs with these nontrivial bounds
are exceedingly rare, going to the trouble of removing them
doesn't seem worth the effort.
Martin
Jeff
