On 1/4/21 1:53 PM, Martin Sebor 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
Right, but that message talks about GC. This is not a GC issue.
This feels like we need a SAVE_EXPR to me to ensure single evaluation
and an unshare_expr to avoid problems with destructive gimplification.
>
>> 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
> }
>
> The front end redeclaration warnings consider all expressions,
> including
>
> int f (void);
>
> void g (int[f () + 1]);
> void g (int[f () + 2]); // warning
>
> The patch turns these complex bounds into strings that the front
> end compares instead.
If you can have an arbitrary expression, such as a function call like
that, then ISTM that a SAVE_EPR is mandatory as you can't call the
function more than once. BUt it also seems to me that for cases that
aren't simple arithmetic of leaf nodes we could just punt. I doubt
we're going to miss significant real world diagnostics by doing that.
> 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.
But I'm not sure that inventing a new method for smuggling the data
around is all that wise or necessary here. I don't see a message from
anyone suggesting that, but I could have missed it.
jeff
