Re: regressions on HEAD

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Sun, Feb 25, 2018 at 03:44:01AM -0800, Christopher Li wrote:
> 
> I am about to ask this. It has been puzzling me
> for a while. You did not send git merge request any more.
> Per our agreement I can't directly apply your patch.

Sigh.
Don't be puzzled. I told you in November that I considered
our agreement as finished [1]. You even replied to it.
I have indeed stopped to send you pull requests since then.

I've stopped to waste my time to send you series and then having
to wait for months to have some feedback about some stylistic
or other surface issues (and rarely more) and then having again
to wait for months that you take the PR.

If you want to use some of the patches I send on the ML,
please use patchwork or whatever suits you the best.

[1] https://marc.info/?l=linux-sparse&m=151052797925158&w=4

> But there is a lot of patches I did comment on looks good
A lot? Really? Isn't that more like 2 or 3 very small series
among the simplest ones?

> > But people are still waiting and seeing these 148K+ useless
> > warnings and you have forced in a patch which:
> > 1) I explained to you why it was wrong
> 
> I have look back to your email. I have answer your concerns.
> 
> Did you write this in the last email I did reply on
> " *eventually* it may very well be the right solution"
> 
> ========== quote from your email ===========
> >
> > Now, I have no hard problem with the PSEUDO_VAL.size
> > approach from a theorical point of view (even if I would
> > prefer the mathematical PoV that 0 is 0, 1 is 1, etc.).
> > I even think that *eventually* it may very well be
> > the right solution (probably the whole typing at IR
> > level need to be revisited).
> 
> I think constant pseudo have a size is the right
> solution in the long run.
> =============== end quote============
> 
> The V2 patch has send out in Nov to address your
> concern that it does not have end to end testing.
> Dibyendu Majumdar was busy and he eventual
> test it come back OK.
> You are CC on the whole thread and no other
> email on that thread. It is 4 month ago.

Absolutely, I wrote that. Note the 'may', the 'eventually', the
'probaly'. Remember the 'longer term' and some 'details' about
CSE and casts (not quoted here).

I didn't replied because your V2 didn't address any of the issues.
It was the same solution with the same deep problems. There was need
for a test to know that. I knew that, you knew that too.
Plus, you didn't did any testing I advised to do (at IR level).

Didn't you understood that if I advised to do these sort of testing
it was because the discussion was sterile and that I knew it would
be the only way you could be convinced that there was some issues:
seeing them with your own eyes?

> > 2) nobody need (even not Dibyendu who, since many weeks, has
> >    taken what he wanted in hiw heavily modified tree)
> 
> Dibyendu did care enough to test it. Thanks him for the effort.

I care enough to have taken a lot of my time (to try) to explain
you why the approach was wrong given the current situation.
Needless to say, I failed.

Testing is very very valuable. And I'm very grateful to people who
take the time to test my things. However, sorry for the truism,
no testing can prove the absence of defects in your code, it can only
show you their presence. Hence the importance of thinking first about
the code and understand it, having a good test coverage, different kind
of testing. Hence my insistence that, *if* you make changes to the
generated IR, you *need* to have good ways of testing at IR level.

Also, testing is the final step, when you can maybe find some bugs
but after the principles have already been validated.
In short: testing doesn't dispense you to think about the code.

> > 3) have not been tested (I told you that *if* you make changes
> >    to the generated code, you better should test it. You have
> >    zero such tests)
> 
> As you know I did have the full kernel compile test on it. It give
> the same result.

A kernel compile is essentially useless to test the generated IR.
You need specific test for it.

> So it is not badly affecting the kernel checking for sure.

That's indeed the case. But since there are regressions in the
optimizations you can have at any time a regressions at the context
checking, for example, as it's sometimes quite sensitive to it.

> > 4) contains a beginner error
> 
> Do you know if that follow up patch can fix it?

I can't test that now. I'll do it later this evening (or tomorrow)
even if I don't doubt that *this* *specific* issue is now fixed.

> If that is the eventually right direction, it has bug
> not as intended, then we fix the bug.
> 
> If it has directional issue, then that is a different
> story.

Chris,
I can't force you to take my word for it but I tell you now,
once again and for the last time, that these two patches have
others issues and that the pseudo-size solution is currently
acceptable because of these issues. I'll be the first one
ready to reevaluate it once the cast rework have been done.

-- Luc "very very tired of this, lasting since March 2107"
--
To unsubscribe from this list: send the line "unsubscribe linux-sparse" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Newbies FAQ]     [LKML]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Trinity Fuzzer Tool]

  Powered by Linux