[PATCH 0/2] Fix illegal simplification of volatile stores

[PATCH 0/2] Fix illegal simplification of volatile stores

[Luc Van Oostenryck]

This series inhibit the illegal simplification of redundant
stores when the stores are volatiles ones.

Luc Van Oostenryck (2):
  add test case for bogus volatile simplification
  fix: volatile stores must not be simplified

 memops.c                     |  6 +++++-
 validation/memops-volatile.c | 15 +++++----------
 2 files changed, 10 insertions(+), 11 deletions(-)

-- 
2.14.0

[PATCH 1/2] add test case for bogus volatile simplification

[Luc Van Oostenryck]

Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
---
 validation/memops-volatile.c | 16 ++++++----------
 1 file changed, 6 insertions(+), 10 deletions(-)

diff --git a/validation/memops-volatile.c b/validation/memops-volatile.c
index 0f3e12ad2..71299f49d 100644
--- a/validation/memops-volatile.c
+++ b/validation/memops-volatile.c
@@ -1,6 +1,7 @@
 static int foo(volatile int *a, int v)
 {
 	*a = v;
+	*a = 0;
 	return *a;
 }
 
@@ -8,14 +9,9 @@ static int foo(volatile int *a, int v)
  * check-name: memops-volatile
  * check-command: test-linearize $file
  *
- * check-output-start
-foo:
-.L0:
-	<entry-point>
-	store.32    %arg2 -> 0[%arg1]
-	load.32     %r5 <- 0[%arg1]
-	ret.32      %r5
-

[PATCH 2/2] fix: volatile stores must not be simplified

[Luc Van Oostenryck]

kill_dominated_stores() simplify away redundant stores.
Nice but volatile stores are never redundant and must
never be simplified away.

Signed-off-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>
---
 memops.c                     | 6 +++++-
 validation/memops-volatile.c | 1 -
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/memops.c b/memops.c
index aeacdf566..1c845a5f9 100644
--- a/memops.c
+++ b/memops.c
@@ -158,8 +158,12 @@ static void kill_dominated_stores(struct basic_block *bb)
 		if (insn->opcode == OP_STORE) {
 			struct instruction *dom;
 			pseudo_t pseudo = insn->src;
-			int local = local_pseudo(pseudo);
+			int local;
+
+			if (insn->type->ctype.modifiers & MOD_VOLATILE)
+				continue;
 
+			local = local_pseudo(pseudo);
 			RECURSE_PTR_REVERSE(insn, dom) {
 				int dominance;
 				if (!dom->bb)
diff --git a/validation/memops-volatile.c b/validation/memops-volatile.c
index 71299f49d..15314e1ce 100644
--- a/validation/memops-volatile.c
+++ b/validation/memops-volatile.c
@@ -9,7 +9,6 @@ static int foo(volatile int *a, int v)
  * check-name: memops-volatile
  * check-command: test-linearize $file
  *
- * check-known-to-fail
  * check-output-ignore
  * check-output-contains: store\\..*%arg2 -> 0\\[%arg1]
  * check-output-contains: store\\..*\\$0 -> 0\\[%arg1]
-- 
2.14.0

Re: [PATCH 0/2] Fix illegal simplification of volatile stores

[Christopher Li]

On Sun, Nov 12, 2017 at 12:24 AM, Luc Van Oostenryck
<luc.vanoostenryck@gmail.com> wrote:
> This series inhibit the illegal simplification of redundant
> stores when the stores are volatiles ones.
>
> Luc Van Oostenryck (2):
>   add test case for bogus volatile simplification
>   fix: volatile stores must not be simplified

Looks good to me.

Can I apply those?

Chris

Re: [PATCH 0/2] Fix illegal simplification of volatile stores

[Luc Van Oostenryck]

On Sun, Nov 12, 2017 at 01:29:08AM +0800, Christopher Li wrote:
> On Sun, Nov 12, 2017 at 12:24 AM, Luc Van Oostenryck
> <luc.vanoostenryck@gmail.com> wrote:
> > This series inhibit the illegal simplification of redundant
> > stores when the stores are volatiles ones.
> >
> > Luc Van Oostenryck (2):
> >   add test case for bogus volatile simplification
> >   fix: volatile stores must not be simplified
> 
> Looks good to me.
> 
> Can I apply those?


If you would do that:
1) it would be equivalent to rebasing one of my branches
2) it make me wonder if I have misunderstood something to the way
   we had, some months ago, agreed to work together.
3) it make me wonder why there is 67 branches in my github
   tree waiting for you to pull them, most having been totally
   ignored by you, a good number of them being 7 or 8 months old
   with a few even older (not talking about the dozens of other
   topic branches that I've never bothered to submit because they
   don't make sense without their parent branches).
4) it make me wonder why this sudden promptitude from your part.


Now, to be honest, I don't care much anymore, I'm just sick and tired of it.

-- Luc

Re: [PATCH 0/2] Fix illegal simplification of volatile stores

[Christopher Li]

On Sun, Nov 12, 2017 at 6:59 AM, Luc Van Oostenryck
<luc.vanoostenryck@gmail.com> wrote:
> On Sun, Nov 12, 2017 at 01:29:08AM +0800, Christopher Li wrote:
>
> If you would do that:
> 1) it would be equivalent to rebasing one of my branches
> 2) it make me wonder if I have misunderstood something to the way
>    we had, some months ago, agreed to work together.

Just asking. Because you start to send patches like the build
series I don't intend to apply. I need to know it fall into the same
category.

> 3) it make me wonder why there is 67 branches in my github
>    tree waiting for you to pull them, most having been totally
>    ignored by you, a good number of them being 7 or 8 months old
>    with a few even older (not talking about the dozens of other
>    topic branches that I've never bothered to submit because they
>    don't make sense without their parent branches).

Hey, I assume you mean the llvm fix series.

We have agreement that will resume the pull again after the 5.0.1.
Which means right now. That series I just send out one important
feed back about the OP_PUSH. I thought I have send that one out
but I just find out it haven't. That is my bad.

Once we finalize a solution for that, that series is pull-able.

Please response to that email.

> 4) it make me wonder why this sudden promptitude from your part.
>

I am confused and need some clarification as well.

Chris

Re: [PATCH 0/2] Fix illegal simplification of volatile stores

[Luc Van Oostenryck]

On Sun, Nov 12, 2017 at 2:14 AM, Christopher Li <sparse@chrisli.org> wrote:
> On Sun, Nov 12, 2017 at 6:59 AM, Luc Van Oostenryck
> <luc.vanoostenryck@gmail.com> wrote:
>> On Sun, Nov 12, 2017 at 01:29:08AM +0800, Christopher Li wrote:
>>
>> If you would do that:
>> 1) it would be equivalent to rebasing one of my branches
>> 2) it make me wonder if I have misunderstood something to the way
>>    we had, some months ago, agreed to work together.
>
> Just asking. Because you start to send patches like the build
> series I don't intend to apply. I need to know it fall into the same
> category.
>
>> 3) it make me wonder why there is 67 branches in my github
>>    tree waiting for you to pull them, most having been totally
>>    ignored by you, a good number of them being 7 or 8 months old
>>    with a few even older (not talking about the dozens of other
>>    topic branches that I've never bothered to submit because they
>>    don't make sense without their parent branches).
>
> Hey, I assume you mean the llvm fix series.

The LLVM fixes series and the 66 others series.

> We have agreement that will resume the pull again after the 5.0.1.
> Which means right now.

Chris ...
v0.5.1 was out mid-August (and you made a very long and painful thing of it).
You ignored the last pull request for 7 weeks.

> That series I just send out one important
> feed back about the OP_PUSH. I thought I have send that one out
> but I just find out it haven't. That is my bad.

Is that serious?
Given how you have been so good the last 2-3 years at ignoring
patches & series, must I, once more, trust you and believe this?

-- Luc

Re: [PATCH 0/2] Fix illegal simplification of volatile stores

[Christopher Li]

On Sun, Nov 12, 2017 at 1:19 PM, Luc Van Oostenryck
<luc.vanoostenryck@gmail.com> wrote:
> The LLVM fixes series and the 66 others series.

I see one pull request for llvm fixes. Where is your other 66
freaking pull request?

> Chris ...
> v0.5.1 was out mid-August (and you made a very long and painful thing of it).

Well it is kind of long and painful due to the bugs we found
at the last stage. What do you expect? ship with the bugs?

> You ignored the last pull request for 7 weeks.

Yes, that is 68 patches it take some time for me to
go through. Didn't the sending patches documents
mention 15 patches at a time?

Yes, I have very limited time to hack on sparse.
But I do have the best intention for sparse.
I do give me my effort to work on sparse with the
resource on my hand. Recently I have been traveling
a lot so my time is very spotted. I haven't been the
best of maintainer and I apologize for that.
I do want to work with you to get this thing merged.

As we agree, I don't pull if I am not complete happy with
the series. Why do you keep refusing to make any change
to your pull request. I even offer I pull into topic branch
and I personally fix it for you on top your change. You have
no reply on that regard.

If you want the code get merged, you need to work with
maintainer. Smearing the maintain and refuse to make any
change does not help to get your code merge. It only
help you to advance your political agenda.

For example, if you drop the unresolved OP_PUSH patches
from the series. I can pull your llvm-fix now.
Do you want to do that? Or that is not an option for you?

>
>> That series I just send out one important
>> feed back about the OP_PUSH. I thought I have send that one out
>> but I just find out it haven't. That is my bad.
>
> Is that serious?

Well not as serious as you don't provide any solutions to resolve
the issue in hand. I am object to the OP_PUSH, that one need more
discussion. Can we have a solution to merge the rest?

> Given how you have been so good the last 2-3 years at ignoring
> patches & series, must I, once more, trust you and believe this?

Luc, being an open source developer, you need to
have an open mind. It has came to my attention you
seems have some dubious intentions of doing things.

Chris

Re: [PATCH 0/2] Fix illegal simplification of volatile stores

[Luc Van Oostenryck]

On Sun, Nov 12, 2017 at 05:57:32PM +0800, Christopher Li wrote:
> On Sun, Nov 12, 2017 at 1:19 PM, Luc Van Oostenryck
> <luc.vanoostenryck@gmail.com> wrote:
> > The LLVM fixes series and the 66 others series.
> 
> I see one pull request for llvm fixes. Where is your other 66
> freaking pull request?
> 
> > Chris ...
> > v0.5.1 was out mid-August (and you made a very long and painful thing of it).
> 
> Well it is kind of long and painful due to the bugs we found
> at the last stage. What do you expect? ship with the bugs?

No Chris. 
We spend a full month discussing about how you was willing to
accept a solution to finally accept my solution and claiming
it was what you meant since the beginning.

> > You ignored the last pull request for 7 weeks.
> 
> Yes, that is 68 patches it take some time for me to
> go through. Didn't the sending patches documents
> mention 15 patches at a time?

The first part of the series: pre-llvm-fixes is 16 patches.
These patches have been submittted in March, 8 months ago.
Is it normal to take 8 months to review 16 patches?

> Yes, I have very limited time to hack on sparse.
> But I do have the best intention for sparse.
> I do give me my effort to work on sparse with the
> resource on my hand. Recently I have been traveling
> a lot so my time is very spotted. I haven't been the
> best of maintainer and I apologize for that.
> I do want to work with you to get this thing merged.
> 
> As we agree, I don't pull if I am not complete happy with
> the series.

This is most probably your worst error.
Months ago, Linus gave you an advice. He said:
	Particularly with somebody like Luc, who sends
	involved patch series for core stuff and knows
	what he is doing, I think Chris could just do
	git merges, and lower the maintenance overhead
	a lot.

You choose to not listen to this advice. It's your right,
of course but choices have consequences.


> Why do you keep refusing to make any change
> to your pull request.

I refuse to make frivolous changes 8 months after a series
been submitted, like you asked last week.
I'm very reluctant to make non-bug related changes 8 months
after a series have been submitted.

The issue with SETVAL having no type have been discussed on
the mailing list in March, there were several patches, several
ideas and opinions. Some patches were dropped but a working
solution emerged. This solution was developped, integrated
with all the others changes, *tested* and was satisfying
to the person involved.
Who didn't participated in these discussions?

> I even offer I pull into topic branch
> and I personally fix it for you on top your change. You have
> no reply on that regard.

You already proposed this some times ago, at least twice IIRC,
and I said that I would be fine with this.
But you must also understand that, given the limited time you
have to accept patches, I have serious doubt about this.

> If you want the code get merged, you need to work with
> maintainer.

If you want to maintain a project, you need to work reasonably
with the developpers. Taking 24 months to take a good series
without having even really reviewed it is not what I call
working reasonably well with devs.
Same here with the 8 months old LLVM series.

> Smearing the maintain and refuse to make any
> change does not help to get your code merge. It only
> help you to advance your political agenda.

Isn't that a bit easy?
I've been very very patient with you, I have spent a lot
of time and energy trying to improve sparse, mainly fixing
bugs. Look at the activity of the last 8 years and look at
the activity of the last 12 months. Improving sparse, this
is my agenda. And you, what have you helped exactly here?

> For example, if you drop the unresolved OP_PUSH patches
> from the series. I can pull your llvm-fix now.
> Do you want to do that? Or that is not an option for you?

It's an option but:
1) You must understand that this represent work.
   Since, you have so limited time, you could:
   - be efficient
   - respect people's time & work.
2) The actual solution is working and tested.
   Removing the OP_PUSH would be a regression.

The correct solution, IMO, is to integrate the current
solution and if you can come with a better one, please do.
Integrate this solution and test it and if it appears to
be a superior solution, replace the actual one with yours.
 
> >
> >> That series I just send out one important
> >> feed back about the OP_PUSH. I thought I have send that one out
> >> but I just find out it haven't. That is my bad.
> >
> > Is that serious?
> 
> Well not as serious as you don't provide any solutions to resolve
> the issue in hand. I am object to the OP_PUSH, that one need more
> discussion. Can we have a solution to merge the rest?
> 
> > Given how you have been so good the last 2-3 years at ignoring
> > patches & series, must I, once more, trust you and believe this?
> 
> Luc, being an open source developer, you need to
> have an open mind. It has came to my attention you
> seems have some dubious intentions of doing things.

Chris, I think you should take a bit time to think about
what you should change to how you work for sparse.
Try to put yourself for a moment in the skin and head
of the developers who have spent time to contribute to sparse,
have submitted some patches. Go ask to these devs why they don't
bother anymore to send patches and to hack on sparse.
Try to list what you expect from a project you want to contribute.

For my part, as I have already said, I'm tired, very very tired
of all this. I also have accumulated way to much patches since
way too long time. And this begin, slowly but surely, to be a
problem for the development I'm doing. I need to have all these
patches consolidated, it's what I'm doing since last week.

-- Luc

Re: [PATCH 0/2] Fix illegal simplification of volatile stores

[Christopher Li]

On Sun, Nov 12, 2017 at 7:10 PM, Luc Van Oostenryck
<luc.vanoostenryck@gmail.com> wrote:
> No Chris.
> We spend a full month discussing about how you was willing to
> accept a solution to finally accept my solution and claiming
> it was what you meant since the beginning.

I have no idea what you are talking about any more.
I recall there are two very serious bugs, one is the
ptrlist iteration while modify. There other one is the
wine deadloop bug. Both this two bus hold up the
release.

You can also blame me for hold up the release too
if that means you feel better.


>
> The first part of the series: pre-llvm-fixes is 16 patches.
> These patches have been submittted in March, 8 months ago.
> Is it normal to take 8 months to review 16 patches?

Luc, per our email exchange you know that one of the
very important pull email was lost. And you said you
believe and I don't need how show you the screen shot
but I did any way.

Why do you pretend you all the sudden forget about
this issue and roll back the time line to 8 month? Just
to make me look bad?

If that is what you want. You win.

Can we go back to get a solution to get the patch merged
start from your llvm-fix series? I can not partial merge
the branch. I do have issue with the OP_PUSH.
That hold up the pull.

You have been very abusive to me on email. That
makes me not want to reply to you. Basically feeding the
trolls.

>
> This is most probably your worst error.
> Months ago, Linus gave you an advice. He said:
>         Particularly with somebody like Luc, who sends
>         involved patch series for core stuff and knows
>         what he is doing, I think Chris could just do
>         git merges, and lower the maintenance overhead
>         a lot.

Come on, what Linus mean was I should use git merge
rather than apply patches. You take it out of context
of comparing applying patch vs git merge. You insert your
own interpretation that I should not try to provide feed back
and fix up with your branch.
I usually tolerate small disagreement on the patches in the
pull request, just suck it up.

The OP_PUSH is too big a change for that. If I merge it
then revert it in the later part of the branch. It is kind of
ugly. So I want to know what is the option I have here
to resolve this issue?

>
> You choose to not listen to this advice. It's your right,
> of course but choices have consequences.

Yes, you can abuse me and justify forking sparse.

>> Why do you keep refusing to make any change
>> to your pull request.
>
> I refuse to make frivolous changes 8 months after a series
> been submitted, like you asked last week.
> I'm very reluctant to make non-bug related changes 8 months
> after a series have been submitted.
>
> The issue with SETVAL having no type have been discussed on
> the mailing list in March, there were several patches, several
> ideas and opinions. Some patches were dropped but a working
> solution emerged. This solution was developped, integrated
> with all the others changes, *tested* and was satisfying
> to the person involved.
> Who didn't participated in these discussions?

Come on. You are CC on the SETVAL patch. Others comments
on it. You ever did. Why?
I join late to the discussion but you are the one did not finish it.

Also why you did not review my other code changes?
I might be late to do it. But I do think careful about your changes.

> You already proposed this some times ago, at least twice IIRC,
> and I said that I would be fine with this.

Then for the OP_PUSH I think that is a much better solutions.
Because until now you fail to express why SETVAL.size patch
is technically inferior to your OP_PUSH. I have evidence to show
otherwise.

If you think the only problem of SETVAL.size is not complete,
why don't admit that? I can provide the rest of the change make
it complete. It take a rebase to cleanly drop OP_PUSH. It would
be best if you can provide a pull point that does not have OP_PUSH.
If I do it, it will means I rebase or revert your patches.
With your blessing, I will go ahead and do it.


> But you must also understand that, given the limited time you
> have to accept patches, I have serious doubt about this.

You can keep your doubt. It is free. Give me your blessing
of me droping OP_PUSH I will show you the code.

> If you want to maintain a project, you need to work reasonably
> with the developpers. Taking 24 months to take a good series
> without having even really reviewed it is not what I call
> working reasonably well with devs.
> Same here with the 8 months old LLVM series.

Good. 8 months old. I am doing nothing in that 8 month.
Not applying one single patch and not releasing one single
bits. Thank you.

Can we going back to find a solutions to merge the bits
quicker? Give me your blessing I can do the dirty work.
Make you the condensing one that review and throw
rock on my branch for reviews. I well come that.

> Isn't that a bit easy?
> I've been very very patient with you, I have spent a lot
> of time and energy trying to improve sparse, mainly fixing
> bugs. Look at the activity of the last 8 years and look at
> the activity of the last 12 months. Improving sparse, this
> is my agenda. And you, what have you helped exactly here?

I am very much appreciate your work and help on sparse.
If that is not very clear to you. I repeat it here.
I am very much appreciate your help. Thank you.

What I am helping you? If you say nothing then I am fine
with that.

>> For example, if you drop the unresolved OP_PUSH patches
>> from the series. I can pull your llvm-fix now.
>> Do you want to do that? Or that is not an option for you?
>
> It's an option but:
> 1) You must understand that this represent work.
>    Since, you have so limited time, you could:
>    - be efficient
>    - respect people's time & work.
> 2) The actual solution is working and tested.
>    Removing the OP_PUSH would be a regression.

The OP_PUSH is too much a issue apply the revert it.
Especially it haven't merge yet.

Need to go, To be continue.

Chris

Re: [PATCH 0/2] Fix illegal simplification of volatile stores

[Luc Van Oostenryck]

On Sun, Nov 12, 2017 at 08:07:03PM +0800, Christopher Li wrote:
> On Sun, Nov 12, 2017 at 7:10 PM, Luc Van Oostenryck
> <luc.vanoostenryck@gmail.com> wrote:
> > No Chris.
> > We spend a full month discussing about how you was willing to
> > accept a solution to finally accept my solution and claiming
> > it was what you meant since the beginning.
> 
> I have no idea what you are talking about any more.
> I recall there are two very serious bugs, one is the
> ptrlist iteration while modify. There other one is the
> wine deadloop bug. Both this two bus hold up the
> release.
> 
> You can also blame me for hold up the release too
> if that means you feel better.
> 
> 
> >
> > The first part of the series: pre-llvm-fixes is 16 patches.
> > These patches have been submittted in March, 8 months ago.
> > Is it normal to take 8 months to review 16 patches?
> 
> Luc, per our email exchange you know that one of the
> very important pull email was lost. And you said you
> believe and I don't need how show you the screen shot
> but I did any way.
> 
> Why do you pretend you all the sudden forget about
> this issue and roll back the time line to 8 month? Just
> to make me look bad?

I didn't at all forgot this and I still believe that at the
time you indeed never saw those pull requests (there was more
than one). But time passes and accumulates.
Even if you didn't saw the pull requests, you saw the posting
for the series, no? And it's since months that you're aware
that you didn't saw the pull request. There is also the mail
archive and, much more useful, patchwork that keep all the
patches. I also sent you a newer pull request 8 weeks ago,
right? And it was ignored for 7 weeks, right?
You could have send an email saying "sorry, I'll need a few weeks
more because now I don't have the time" but no, just nothing.

So, no, I don't roll back time to 8 months, it's just that I'm
waiting since 8 months to have these patches handled by you.

> Can we go back to get a solution to get the patch merged
> start from your llvm-fix series? I can not partial merge
> the branch. I do have issue with the OP_PUSH.
> That hold up the pull.

OK, that have at least the merit to be very clear.
 
> You have been very abusive to me on email. That
> makes me not want to reply to you. Basically feeding the
> trolls.
> 
> >
> > This is most probably your worst error.
> > Months ago, Linus gave you an advice. He said:
> >         Particularly with somebody like Luc, who sends
> >         involved patch series for core stuff and knows
> >         what he is doing, I think Chris could just do
> >         git merges, and lower the maintenance overhead
> >         a lot.
> 
> Come on, what Linus mean was I should use git merge
> rather than apply patches. You take it out of context
> of comparing applying patch vs git merge. You insert your
> own interpretation that I should not try to provide feed back
> and fix up with your branch.

I copied his words verbatim.
You seem to be very sure of what he meant, fine.

> I usually tolerate small disagreement on the patches in the
> pull request, just suck it up.
> 
> The OP_PUSH is too big a change for that. If I merge it
> then revert it in the later part of the branch. It is kind of
> ugly. So I want to know what is the option I have here
> to resolve this issue?
> 
> >
> > You choose to not listen to this advice. It's your right,
> > of course but choices have consequences.
> 
> Yes, you can abuse me and justify forking sparse.
> 
> >> Why do you keep refusing to make any change
> >> to your pull request.
> >
> > I refuse to make frivolous changes 8 months after a series
> > been submitted, like you asked last week.
> > I'm very reluctant to make non-bug related changes 8 months
> > after a series have been submitted.
> >
> > The issue with SETVAL having no type have been discussed on
> > the mailing list in March, there were several patches, several
> > ideas and opinions. Some patches were dropped but a working
> > solution emerged. This solution was developped, integrated
> > with all the others changes, *tested* and was satisfying
> > to the person involved.
> > Who didn't participated in these discussions?
> 
> Come on. You are CC on the SETVAL patch. Others comments
> on it. You ever did. Why?
> I join late to the discussion but you are the one did not finish it.
> 
> Also why you did not review my other code changes?

Because I don't comment on code that I somehow disagree with,
or is not to my taste but which is working well enough that
I don't want to block.

> I might be late to do it. But I do think careful about your changes.
> 
> > You already proposed this some times ago, at least twice IIRC,
> > and I said that I would be fine with this.
> 
> Then for the OP_PUSH I think that is a much better solutions.
> Because until now you fail to express why SETVAL.size patch
> is technically inferior to your OP_PUSH. I have evidence to show
> otherwise.

In my world, evidences are patches, working patches, integrated
with the rest of the code and reasonably tested.
I have a solution which do that, it has been submitted and used
by me and Dibyendu. You claim to have a superior solution, OK.
Where are the patches? Where can we see the whole thing integrated
with all the others fixes? On what have this been tested?

> If you think the only problem of SETVAL.size is not complete,
> why don't admit that? I can provide the rest of the change make
> it complete. It take a rebase to cleanly drop OP_PUSH. It would
> be best if you can provide a pull point that does not have OP_PUSH.
> If I do it, it will means I rebase or revert your patches.
> With your blessing, I will go ahead and do it.
> 
> 
> > But you must also understand that, given the limited time you
> > have to accept patches, I have serious doubt about this.
> 
> You can keep your doubt. It is free. Give me your blessing
> of me droping OP_PUSH I will show you the code.

The code is there Chris, it has been submitted, it has my SoB and
thus agree with the license. You have fully the right to take my
code, you don't need my blessing.
If you want to take the code I wrote, take some bits and drop some
others and if you think it's a good idea, that's how you can be
useful to sparse, fine, please do.

> > If you want to maintain a project, you need to work reasonably
> > with the developpers. Taking 24 months to take a good series
> > without having even really reviewed it is not what I call
> > working reasonably well with devs.
> > Same here with the 8 months old LLVM series.
> 
> Good. 8 months old. I am doing nothing in that 8 month.
> Not applying one single patch and not releasing one single
> bits. Thank you.
> 
> Can we going back to find a solutions to merge the bits
> quicker?

So, now it's me that is holding things? Incredible.

> Give me your blessing I can do the dirty work.
> Make you the condensing one that review and throw
> rock on my branch for reviews. I well come that.
>
> > Isn't that a bit easy?
> > I've been very very patient with you, I have spent a lot
> > of time and energy trying to improve sparse, mainly fixing
> > bugs. Look at the activity of the last 8 years and look at
> > the activity of the last 12 months. Improving sparse, this
> > is my agenda. And you, what have you helped exactly here?
> 
> I am very much appreciate your work and help on sparse.
> If that is not very clear to you. I repeat it here.
> I am very much appreciate your help. Thank you.
> 
> What I am helping you? If you say nothing then I am fine
> with that.
> 
> >> For example, if you drop the unresolved OP_PUSH patches
> >> from the series. I can pull your llvm-fix now.
> >> Do you want to do that? Or that is not an option for you?
> >
> > It's an option but:
> > 1) You must understand that this represent work.
> >    Since, you have so limited time, you could:
> >    - be efficient
> >    - respect people's time & work.
> > 2) The actual solution is working and tested.
> >    Removing the OP_PUSH would be a regression.
> 
> The OP_PUSH is too much a issue apply the revert it.
> Especially it haven't merge yet.

OK, fine. Again, at least it's very clear.
 
-- Luc

Re: [PATCH 0/2] Fix illegal simplification of volatile stores

[Christopher Li]

On Mon, Nov 13, 2017 at 2:48 AM, Luc Van Oostenryck
<luc.vanoostenryck@gmail.com> wrote:
> I didn't at all forgot this and I still believe that at the
> time you indeed never saw those pull requests (there was more
> than one). But time passes and accumulates.
> Even if you didn't saw the pull requests, you saw the posting
> for the series, no? And it's since months that you're aware
> that you didn't saw the pull request. There is also the mail
> archive and, much more useful, patchwork that keep all the
> patches. I also sent you a newer pull request 8 weeks ago,
> right? And it was ignored for 7 weeks, right?
> You could have send an email saying "sorry, I'll need a few weeks
> more because now I don't have the time" but no, just nothing.

Luc, that is what I did. Didn't you receive this email on Oct 17:
=======================================
Yes, I have the off line review write up. Let me send it out.

Thanks for the reminding.

Chris
=======================================

What kind of games you are trying to play here?

The reason those email is hard to write, even after I have
the offline review is that. I know for a fact that you are going
to make a big deal of OP_PUSH.

It is not about the technical merit any more. It is about
Chris is not a good maintainer there for Luc should do
whatever he want.

You are making your series not touchable. Amount all
all patch email send over the mailing list. How much can
I actually pull from according our agreement? Zero.

There is only one valid git pull request, the llvm-fix one.
It is hold up by the OP_PUSH. I have full screen of
patch you send I can't apply.

>> Come on, what Linus mean was I should use git merge
>> rather than apply patches. You take it out of context
>> of comparing applying patch vs git merge. You insert your
>> own interpretation that I should not try to provide feed back
>> and fix up with your branch.
>
> I copied his words verbatim.
> You seem to be very sure of what he meant, fine.

Of course. Stop using Linus email as justification of forcing
your code in, even when it is questionable not having technical
merit.

The review and merge process is design to keep the bad bits
out. You are handicapping me by insist on refuse to make any
fix up changes.

>
> Because I don't comment on code that I somehow disagree with,
> or is not to my taste but which is working well enough that
> I don't want to block.

So it is not about technical merit any more. It is about some
one you disagree with.

And what is the reason for not reviewing this:
"Simplify expr->flags assignement":

https://patchwork.kernel.org/patch/10042019/

You are basically starving my patches so that you can claim
"patch proposed and dropped".

Luc, you have make it personal. You need to be more open
minded with the person you have technical disagreement
with. The more you do this kind of stuff the more it shows
that you are not ready to become a good open source maintainer.

>
> In my world, evidences are patches, working patches, integrated
> with the rest of the code and reasonably tested.
> I have a solution which do that, it has been submitted and used
> by me and Dibyendu. You claim to have a superior solution, OK.
> Where are the patches? Where can we see the whole thing integrated
> with all the others fixes? On what have this been tested?

Come on, I want to get some feed back from you first before
I develop that into a full working series. Since you are the one
insist on type-less constant so you might able to see some reasoning
I don't. So far you provide none.

Fine, I will follow up with this one.

> The code is there Chris, it has been submitted, it has my SoB and
> thus agree with the license. You have fully the right to take my
> code, you don't need my blessing.
> If you want to take the code I wrote, take some bits and drop some
> others and if you think it's a good idea, that's how you can be
> useful to sparse, fine, please do.

Of course I don't want to partial apply the series.
But I do have technical reason to hold of the OP_PUSH.
What is the solution you suggest here if I don't agree with
OP_PUSH?

I can of course follow up with the PSEUDO_VAL.size one.
But what about your other part of the series. Can you purpose
a different pull request without OP_PUSH?

Let's discuss a working model I can do my job rejecting some
patch if really necessary. How many email have we exchange
and not having a solution?

>
> So, now it's me that is holding things? Incredible.

Yes, by refusing to get a workable solutions if I reject OP_PUSH.

Am I not allow to reject the OP_PUSH because it lack of
technical merit?

The whole reason I am hesitate to send out that OP_PUSH
review is because I know you will this kind of behavior.

Chris

Re: [PATCH 0/2] Fix illegal simplification of volatile stores

[Luc Van Oostenryck]

On Mon, Nov 13, 2017 at 06:10:26AM +0800, Christopher Li wrote:
> On Mon, Nov 13, 2017 at 2:48 AM, Luc Van Oostenryck
> <luc.vanoostenryck@gmail.com> wrote:
> > I didn't at all forgot this and I still believe that at the
> > time you indeed never saw those pull requests (there was more
> > than one). But time passes and accumulates.
> > Even if you didn't saw the pull requests, you saw the posting
> > for the series, no? And it's since months that you're aware
> > that you didn't saw the pull request. There is also the mail
> > archive and, much more useful, patchwork that keep all the
> > patches. I also sent you a newer pull request 8 weeks ago,
> > right? And it was ignored for 7 weeks, right?
> > You could have send an email saying "sorry, I'll need a few weeks
> > more because now I don't have the time" but no, just nothing.
> 
> Luc, that is what I did. Didn't you receive this email on Oct 17:
> =======================================
> Yes, I have the off line review write up. Let me send it out.
> 
> Thanks for the reminding.
> 
> Chris
> =======================================

But did you sent this review out? When?
 
> What kind of games you are trying to play here?
> 
> The reason those email is hard to write, even after I have
> the offline review is that. I know for a fact that you are going
> to make a big deal of OP_PUSH.

No. I've clearly expressed that I'm quite tired.
I have no intention to make any fight about this OP_PUSH.
 
> It is not about the technical merit any more. It is about
> Chris is not a good maintainer there for Luc should do
> whatever he want.

I have already explained that giving some review in a reasonable
amount of time after a series is first submitted is one thing.
Asking for cosmetic changes (or what I tend to call "surface"
changes) months later is another thing.
The difference being that new code is developed on top
of the old one. And yes, it's always possible to rebase
everything, when it's valuable functional changes.

> You are making your series not touchable. Amount all
> all patch email send over the mailing list. How much can
> I actually pull from according our agreement? Zero.

I consider our agreement as finished.
Pull anything you like.
 
> There is only one valid git pull request, the llvm-fix one.
> It is hold up by the OP_PUSH. I have full screen of
> patch you send I can't apply.

The reason why I insisted about the llvm series is that
because it's the oldest and a lot of further development
depends on it.

> >> Come on, what Linus mean was I should use git merge
> >> rather than apply patches. You take it out of context
> >> of comparing applying patch vs git merge. You insert your
> >> own interpretation that I should not try to provide feed back
> >> and fix up with your branch.
> >
> > I copied his words verbatim.
> > You seem to be very sure of what he meant, fine.
> 
> Of course. Stop using Linus email as justification of forcing
> your code in, even when it is questionable not having technical
> merit.
> 
> The review and merge process is design to keep the bad bits
> out. You are handicapping me by insist on refuse to make any
> fix up changes.
> 
> >
> > Because I don't comment on code that I somehow disagree with,
> > or is not to my taste but which is working well enough that
> > I don't want to block.
> 
> So it is not about technical merit any more. It is about some
> one you disagree with.

You seem to have missed the word "code" here above.
I'll rephrase:
   I don't comment on code if I disagree with the code
   or if the code is not to my taste but still I find
   the code valuable enough that I don't want to block
   it's inclusion.
 
> And what is the reason for not reviewing this:
> "Simplify expr->flags assignement":
> 
> https://patchwork.kernel.org/patch/10042019/

That's exactly the kind of code I find completly without any
value:
- if you make some measurement, please add the mean value
  *AND* the standard deviation
- it's a change for some "optimization" but you state yourself
  in the mail that you don't know if it make any kind of difference.

I suppose, you find it valuable enough, then let's agree we disagree
and since I really don't care about this code and since to me this
change has no "positive" value, it also has no real negative value.
So, I don't comment on this code saying it has no value.

> You are basically starving my patches so that you can claim
> "patch proposed and dropped".

I starving nothing at all.
I would if I would have somehow nacked your patch.

> Luc, you have make it personal. You need to be more open
> minded with the person you have technical disagreement
> with. The more you do this kind of stuff the more it shows
> that you are not ready to become a good open source maintainer.

You see personnal things where there isn't.
 
> > In my world, evidences are patches, working patches, integrated
> > with the rest of the code and reasonably tested.
> > I have a solution which do that, it has been submitted and used
> > by me and Dibyendu. You claim to have a superior solution, OK.
> > Where are the patches? Where can we see the whole thing integrated
> > with all the others fixes? On what have this been tested?
> 
> Come on, I want to get some feed back from you first before
> I develop that into a full working series. Since you are the one
> insist on type-less constant so you might able to see some reasoning
> I don't. So far you provide none.

I don't insist on anything. You have said yourself that I gave no
arguments and it's kinda true.
What I have is a working, integrated and tested set of patches.
 
> Fine, I will follow up with this one.
> 
> > The code is there Chris, it has been submitted, it has my SoB and
> > thus agree with the license. You have fully the right to take my
> > code, you don't need my blessing.
> > If you want to take the code I wrote, take some bits and drop some
> > others and if you think it's a good idea, that's how you can be
> > useful to sparse, fine, please do.
> 
> Of course I don't want to partial apply the series.
> But I do have technical reason to hold of the OP_PUSH.
> What is the solution you suggest here if I don't agree with
> OP_PUSH?

As I said here above: make your own patches with your own solution,
integrate them with the rest of the code and test it.
And when you will have a something working well enough, post them
on the ML so that people can see it and test it on their side.

> I can of course follow up with the PSEUDO_VAL.size one.
> But what about your other part of the series. Can you purpose
> a different pull request without OP_PUSH?

I could but why for? This LLVM series is important, a lot
of my changes depends on it.
Let's first solve this issue with the OP_PUSH.

> Let's discuss a working model I can do my job rejecting some
> patch if really necessary. How many email have we exchange
> and not having a solution?
> 
> >
> > So, now it's me that is holding things? Incredible.
> 
> Yes, by refusing to get a workable solutions if I reject OP_PUSH.

I only ask that you replace it with your solution that you claim
is superior.
Do you maybe expect that I implement your ideas?

> Am I not allow to reject the OP_PUSH because it lack of
> technical merit?

You're the maintainer, you have all the rights you want about
which the code is accepted or not.

> The whole reason I am hesitate to send out that OP_PUSH
> review is because I know you will this kind of behavior.

Funny thing when you have yourself said in late July:
	Warning ahead, the OP_PUSH need more discussion.
	It is not going to apply very quickly.

To which I replied:
	The fact that you said in advance, without any
	kind of discussion, and without having participated
	in any sort of way in the initial discussion, that
	it will be a slow thing (thus a painful thing) make me
	just want to drop the whole thing and move on with
	something else.


I've now spend enough time with this and this is my last email
on the whole subject.

-- Luc Van Oostenryck

Re: [PATCH 0/2] Fix illegal simplification of volatile stores

[Christopher Li]

On Mon, Nov 13, 2017 at 7:06 AM, Luc Van Oostenryck
<luc.vanoostenryck@gmail.com> wrote:
>
> But did you sent this review out? When?
>

I start to send review for this series out at Oct 30 and Nov 5.

For this peculiarity OP_PUSH, I thought I have send out already
but I just find a few days ago I did not. That was my bad.

But the whole process of evaluating your OP_PUSH start much
earlier. Not only do I join the discussion of the thread that
cause the OP_PUSH in the first place. I also purpose the
alternative patch at Aug 13.
https://patchwork.kernel.org/patch/9897573/

That is before you send out the git pull for llvm-fix at Sep.

You drop out of the email discussion.
There is many level of participation of the review process.
Email discussion is one. Sending out patches are the higher
level of participation.

Those email are not directly response to your llvm-fix.
But you know very well those are alternative approach to
OP_PUSH. You make claim that I did not response to
your git pull request for 8 month and you know that is not true.
It just more mud on the wall.

> I have already explained that giving some review in a reasonable
> amount of time after a series is first submitted is one thing.
> Asking for cosmetic changes (or what I tend to call "surface"
> changes) months later is another thing.
> The difference being that new code is developed on top
> of the old one. And yes, it's always possible to rebase
> everything, when it's valuable functional changes.

But the OP_PUSH vs OP_SETVAL.size is not surface change.
Your OP_PUSH break the byte code compatibility and
increase the instruction count.

I would let the surface change slide and just merge it
consider you have other change depend on it.

>
>> You are making your series not touchable. Amount all
>> all patch email send over the mailing list. How much can
>> I actually pull from according our agreement? Zero.
>
> I consider our agreement as finished.

Per our agreement. If I am not complete happy about any
thing in the series. The solutions is I don't pull and you are
going to purpose a new git pull request address my concerns.

Given that you have patch depend on it and I have this
in my backlog for a long time. I am willing to help out and
create fix up patches.

I do care about make things easy for you. But I have to make
a stand on some critical issue like OP_PUSH.

>
> The reason why I insisted about the llvm series is that
> because it's the oldest and a lot of further development
> depends on it.

I understand the pain. But is it possible for you to pick a stable
point as your base like master? In the old days the master
update not very frequently. Now days master are update
reasonably. Wouldn't master serve better as base for the
topic branch?

>
> You seem to have missed the word "code" here above.
> I'll rephrase:
>    I don't comment on code if I disagree with the code
>    or if the code is not to my taste but still I find
>    the code valuable enough that I don't want to block
>    it's inclusion.

That is just a cryptic way of saying I don't like your code.

>
>> And what is the reason for not reviewing this:
>> "Simplify expr->flags assignement":
>>
>> https://patchwork.kernel.org/patch/10042019/
>
> That's exactly the kind of code I find completly without any
> value:
> - if you make some measurement, please add the mean value
>   *AND* the standard deviation
> - it's a change for some "optimization" but you state yourself
>   in the mail that you don't know if it make any kind of difference.

OK. That establish you did saw  the email and chose not
to response to it.

Let me ask this question. If this exact patch is not send by me,
It is send by let say  Ramsay Jones. Would it be much more likely
you did reply to the email? I think so. After all, that did match you
behavior pattern promptly reply to patch by others just not me.

That is the different between you and me. I did not response
due to lack of time. I treat your patch the same way as mine.
If I am not happy about it, I want to make it better. You ignore patch
intentionally and has strong correlation base on me send the patch or not.

And you are still saying this is not personal?

> I don't insist on anything. You have said yourself that I gave no
> arguments and it's kinda true.
> What I have is a working, integrated and tested set of patches.

Thanks. Will do then.

> Funny thing when you have yourself said in late July:
>         Warning ahead, the OP_PUSH need more discussion.
>         It is not going to apply very quickly.
>
> To which I replied:
>         The fact that you said in advance, without any
>         kind of discussion, and without having participated
>         in any sort of way in the initial discussion, that
>         it will be a slow thing (thus a painful thing) make me
>         just want to drop the whole thing and move on with
>         something else.
>
>
> I've now spend enough time with this and this is my last email
> on the whole subject.

Guilty.

That is why I did dig out the discussion and have exchange
some email with you.  I point out your mistake of using earler
Linus' email to counter his own later suggestion.
Your don't have an validate argument and you are the last one
did not response to my reasoning.

Further more, I send out alternative patch to match my suggestion.
You did not follow up.

Luc, I do have a lot of respect for you. Thank you for your contribution
to sparse.

I do want to merge your patches. I do want to do it in a way
that is less painful for you.

I might be a moron of insist certain things at times. Actually,
remove the might, I am.

I do mean to do good with sparse with the best intentions.
I don't want to screw you and I don't want to fork sparse.

Let me see what I can do with the OP_SETVAL.size.

Chris