Chandler,
where we disagree is in whether the current project is moving the
issue
forward. It is not. It is making the compiler more complex for no additional
value.
The current project is not based in evidence, I have asked for any SPEC
benchmark
that shows performance gain by the compiler taking advantage of “undefined
behavior”
and no one can show that.
The current project doesn’t even address some of the bugs described in the
paper,
in particular those derived from the incorrect definition of “undef” in the
LangRef.
The current project perpetuates the myth that “poison” is somehow required.
It isn’t, and when I show proof of that you reply with “its in bug reports,
etc”,
that’s BS and you know it, this hasn’t been explored. The same thing happened
before when I said “nsw” shouldn’t be hoisted, folks replied with “that’s
already
been studied and rejected”, but I did the research and found out no, it had not
been fully explored, Dan discarded the idea based on incorrect analysis and
people
never questioned it. Dan created “poison” on a whim, and people picked up on it
too
without question. We’ve been stuck with this self-inflicted wound ever since,
and it is
time to heal it.
The entire situation here can be summarized as incorrect analysis, and failure
to
fully root-cause problems, because people didn’t question their assumptions.
And we should not be surprised, it is one of the most common problems in
software
engineering. Haven’t you ever gone in to fix a bug only to find that what you
are doing
is correcting someone else’s bug fix that didn’t actually fix anything because
the person
doing the fix didn’t fully understand the problem? Happens to me all the time.
The correct software engineering decision here is to fix the definition of
“undef”,
delete “poison”, and not hoist “nsw” attributes. That is a no-brainer. There
is nothing
to try out, or test, ormeasure. That is simply the way it has to be to avoid the
current
set of problems.
I cannot emphasize that last point enough, fixing the definition of “undef”,
deleting
“poison”, and not allowing “nsw” attributes to be hoisted, fixes all known
problems,
even including ones that weren’t thought of before these discussions started,
and
I don’t think there is any technical disagreement here, not even from John,
Sanjoy,
or Nuno. This is a no-brainer.
John and I do not have a technical disagreement, John is having an emotional
reaction to the fact that he doesn’t have an answer to the function-inlining
question.
We can’t disagree until he actually has an opinion to disagree with, but he is
not
willing to express one.
The work on “poison” and “freeze” should be in new analysis and transform passes
in which folks can do whatever they want to propagate “undefined behavior”
around
in a function (but “undefined behavior" should be an analysis attribute not
an IR
instruction). Then folks can try to see if they can actually measure a
performance gain
on any SPEC benchmarks. I think we all know this is going to turn out negative,
but that’s
besides the point, the point is that “poison” and “freeze” are an experiment and
need
to be treated as such and not just simply forced into the trunk because someone
likes it.
What you are saying about ’the llvm way" goes against everything we know
about
"software engineering”, that things have to be evidence based, have
utility, and
be the least complex solution to avoid confusion. And we do engineering reviews
and take review feedback seriously. I suggest you take a step back and think
about
that, because it sure seems to me like you’re advocating that we don’t do
reviews and
we don’t base decisions on evidence.
Peter Lawrence.
> On Jun 28, 2017, at 12:16 PM, Chandler Carruth <chandlerc at
gmail.com> wrote:
>
> On Wed, Jun 28, 2017 at 9:39 AM Peter Lawrence <peterl95124 at
sbcglobal.net <mailto:peterl95124 at sbcglobal.net>> wrote:
>
>
> Part I.
>
> The original LangRef appeared to be “nice and pretty”
> and originally ‘undef’ did not seem to stick out.
>
> Then evidence came to light in the form of bug reports, and
> in the form of Dan Gohman’s email “the nsw story”, that all
> was not good for ‘undef’ [1,2].
>
> A proposal was made based on that evidence.
> A presentation was made at an llvm gathering.
> A paper was written. The proposal has even been
> implemented and tested.
>
> The IR might not be so “nice and pretty” anymore,
> but at least all the known bugs could be closed.
> I think everyone agreed that the case could be closed
> based on the evidence at hand.
>
> However new evidence has come to light,
> the function-inlining example for one,
> which the proposal does not address.
>
> This means the case must be re-opened.
>
> Peter,
>
> People have been continuing to work on these issues for years. This is not
new, and and it is not only now being reopened.
>
>
> Unfortunately, at this point I think you are repeating well known and well
understood information in email after email. I don't think that is a
productive way to discuss this. However, I don't want to dissuade you from
contributing to the project. But I don't think new emails on this subject
will not be a good use of anyone's time.
>
> Instead, someone needs to go do the very hard work of building, testing,
and understanding solutions to some of these problems. In fact, a few others are
already doing exactly this.
>
> I understand you disagree with the approach others are taking, and that is
perfectly fine, even good! You have explained your concern, and there remains a
technical disagreement. This is OK. Repeating your position won't really
help move forward.
>
> Contributing technical perspectives (especially different ones!) is always
valuable, and I don't want to ever discourage it. But when there remains a
strong technical disagreement, we have to find some way to make
progress.Typically, LLVM lends weight towards those who have the most
significant contributions to LLVM in the area *and* are actually doing the work
to realize a path forward. This doesn't make them any more "right"
or their opinions "better". It is just about having a path forward.
>
> But this should also show you how to make progress. Continuing to debate in
email probably won't help as you're relatively new to the LLVM project.
Instead, write the code, get the data and benchmark results to support your
position and approach, and come back to us. I think you will have to do the
engineering work of building the solution you want (and others disagree with)
and showing why it is objectively better.
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20170628/0588c54a/attachment-0001.html>
On 28 June 2017 at 15:33, Peter Lawrence via llvm-dev <llvm-dev at lists.llvm.org> wrote:> The correct software engineering decision here is to fix the definition of > “undef”, delete “poison”, and not hoist “nsw” attributes. That is a no-brainer. > There is nothing to try out, or test, ormeasure. That is simply the way it has to be to avoid > the current set of problems.Declaring victory and expecting everyone in a foreign community to fall into line is probably the the second-least productive approach that could possibly be taken in this situation. Surpassed only by actually insulting a regular at the same time. Tim.
On Wed, Jun 28, 2017 at 3:33 PM Peter Lawrence <peterl95124 at sbcglobal.net> wrote:> Chandler, > where we disagree is in whether the current project is > moving the issue > forward. It is not. It is making the compiler more complex for no > additional value. >I mean, I also disagree with your analysis of where we disagree, and what is happening, but I don't think that matters much or will convince you of anything.> The current project perpetuates the myth that “poison” is somehow required. >No one thinks it is required at a theoretical level. Current transformations made by LLVM require it. We could always disable those transformations. The current project is attempting to see if there is a pragmatic set of transformations we can keep with a better definition.> It isn’t, and when I show proof of that you reply with “its in bug > reports, etc”, > that’s BS and you know it, this hasn’t been explored. >I'm sorry that I didn't have readily available citations in the other thread. If you can show what searches you have done that didn't find any results, I'm happy to try and help find them. But the way you say this doesn't come across as assuming good faith on the part of myself and others in these discussions. Within the LLVM community, please always assume good faith and don't accuse people of "BS".> Dan created “poison” on a whim, and people picked up on it too > without question. We’ve been stuck with this self-inflicted wound ever > since, and it is > time to heal it. >The way you have described this comes across as both hyperbolic and insulting to specific individuals. This kind of behavior and rhetoric is not acceptable in the LLVM community, and multiple people have asked you to change tone and avoid this behavior. Bluntly, stop. John and I do not have a technical disagreement, John is having an> emotional > reaction to the fact that he doesn’t have an answer to the > function-inlining question. >This is yet another personal attack in your email. After talking to several people involved in these threads, I think that whatever technical points you are trying to make have been completely lost due to the repeated pattern of apparent hostility. Your emails are perceived as insulting and attacking people which is not an appropriate or professional way to engage in a technical discussion here. The attitude and approach you are taking on the list is completely incompatible with this community and project. I still encourage you to modify LLVM and implement your approach. It is open source and easy to fork on GitHub. However, please don't continue sending emails to LLVM lists until you can do so without repeating this behavior. -Chandler -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20170629/1dcaf153/attachment.html>
Tim,
Can you make a case for why this isn’t the right thing to do ?
I think this is a no-brainer because this is the situation we would be
in if folks hadn’t gone off the deep end with “poison” which hasn’t
actually solved any problems (other than the mis-adventure with
“+nsw” which never actually needed “poison” in the first place).
And AFAICT this is the situation that gcc is in, it gets along just fine
without “poison”.
If you think there is a problem that this approach doesn’t solve then
you need to demonstrate it.
Peter.
> On Jun 28, 2017, at 4:01 PM, Tim Northover <t.p.northover at
gmail.com> wrote:
>
> On 28 June 2017 at 15:33, Peter Lawrence via llvm-dev
> <llvm-dev at lists.llvm.org> wrote:
>> The correct software engineering decision here is to fix the definition
of
>> “undef”, delete “poison”, and not hoist “nsw” attributes. That is a
no-brainer.
>> There is nothing to try out, or test, ormeasure. That is simply the way
it has to be to avoid
>> the current set of problems.
>
> Declaring victory and expecting everyone in a foreign community to
> fall into line is probably the the second-least productive approach
> that could possibly be taken in this situation. Surpassed only by
> actually insulting a regular at the same time.
>
> Tim.
Chandler,
I am puzzled by the statement "Current transformations made
by LLVM require it.”
Can you clarify with a C source code example ?
Peter Lawrence.
> On Jun 28, 2017, at 6:47 PM, Chandler Carruth <chandlerc at
gmail.com> wrote:
>
> On Wed, Jun 28, 2017 at 3:33 PM Peter Lawrence <peterl95124 at
sbcglobal.net <mailto:peterl95124 at sbcglobal.net>> wrote:
> Chandler,
> where we disagree is in whether the current project is
moving the issue
> forward. It is not. It is making the compiler more complex for no
additional value.
>
> I mean, I also disagree with your analysis of where we disagree, and what
is happening, but I don't think that matters much or will convince you of
anything.
>
> The current project perpetuates the myth that “poison” is somehow required.
>
> No one thinks it is required at a theoretical level. Current
transformations made by LLVM require it. We could always disable those
transformations. The current project is attempting to see if there is a
pragmatic set of transformations we can keep with a better definition.
>
> It isn’t, and when I show proof of that you reply with “its in bug reports,
etc”,
> that’s BS and you know it, this hasn’t been explored.
>
> I'm sorry that I didn't have readily available citations in the
other thread. If you can show what searches you have done that didn't find
any results, I'm happy to try and help find them. But the way you say this
doesn't come across as assuming good faith on the part of myself and others
in these discussions. Within the LLVM community, please always assume good faith
and don't accuse people of "BS".
>
> Dan created “poison” on a whim, and people picked up on it too
> without question. We’ve been stuck with this self-inflicted wound ever
since, and it is
> time to heal it.
>
> The way you have described this comes across as both hyperbolic and
insulting to specific individuals.
>
> This kind of behavior and rhetoric is not acceptable in the LLVM community,
and multiple people have asked you to change tone and avoid this behavior.
Bluntly, stop.
>
> John and I do not have a technical disagreement, John is having an
emotional
> reaction to the fact that he doesn’t have an answer to the
function-inlining question.
>
> This is yet another personal attack in your email.
>
> After talking to several people involved in these threads, I think that
whatever technical points you are trying to make have been completely lost due
to the repeated pattern of apparent hostility. Your emails are perceived as
insulting and attacking people which is not an appropriate or professional way
to engage in a technical discussion here.
>
> The attitude and approach you are taking on the list is completely
incompatible with this community and project.
>
> I still encourage you to modify LLVM and implement your approach. It is
open source and easy to fork on GitHub. However, please don't continue
sending emails to LLVM lists until you can do so without repeating this
behavior.
>
> -Chandler
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20170628/d54b1466/attachment.html>
On Wed, Jun 28, 2017 at 3:33 PM, Peter Lawrence via llvm-dev < llvm-dev at lists.llvm.org> wrote:> Chandler, > where we disagree is in whether the current project is > moving the issue > forward. It is not. It is making the compiler more complex for no > additional value. > > The current project is not based in evidence, I have asked for any SPEC > benchmark > that shows performance gain by the compiler taking advantage of “undefined > behavior” > and no one can show that. >One easy way to measure this is to enable -fwrapv/-ftrapv on the compiler command line, which impede the compiler's usual exploitation of signed wrap UB in C. Last I heard, those options lead to substantial performance regressions. It sounds like you are implicitly claiming that there would be no performance regression. If the -fwrapv/-ftrapv measurements are in line with this, that will be useful new information for the discussion. But I think that you need to do the work here (and it isn't that much work compared to writing out emails; or perhaps I'm just a slow email writer). I don't think anybody has really up to date numbers on the performance effect of those options on SPEC. Could you please get some up to date numbers before continuing this discussion? If the results are as you seem to imply, then that will be very convincing support for your point. If you can't be bothered to do that, I have to question how invested you are in pushing the LLVM project toward a better solution to its current woes with the poison/undef situation. -- Sean Silva> The current project doesn’t even address some of the bugs described in the > paper, > in particular those derived from the incorrect definition of “undef” in > the LangRef. > > The current project perpetuates the myth that “poison” is somehow required. > It isn’t, and when I show proof of that you reply with “its in bug > reports, etc”, > that’s BS and you know it, this hasn’t been explored. The same thing > happened > before when I said “nsw” shouldn’t be hoisted, folks replied with “that’s > already > been studied and rejected”, but I did the research and found out no, it > had not > been fully explored, Dan discarded the idea based on incorrect analysis > and people > never questioned it. Dan created “poison” on a whim, and people picked up > on it too > without question. We’ve been stuck with this self-inflicted wound ever > since, and it is > time to heal it. > > The entire situation here can be summarized as incorrect analysis, and > failure to > fully root-cause problems, because people didn’t question their > assumptions. > And we should not be surprised, it is one of the most common problems in > software > engineering. Haven’t you ever gone in to fix a bug only to find that what > you are doing > is correcting someone else’s bug fix that didn’t actually fix anything > because the person > doing the fix didn’t fully understand the problem? Happens to me all the > time. > > The correct software engineering decision here is to fix the definition of > “undef”, > delete “poison”, and not hoist “nsw” attributes. That is a no-brainer. > There is nothing > to try out, or test, ormeasure. That is simply the way it has to be to > avoid the current > set of problems. > > I cannot emphasize that last point enough, fixing the definition of > “undef”, deleting > “poison”, and not allowing “nsw” attributes to be hoisted, fixes all known > problems, > even including ones that weren’t thought of before these discussions > started, and > I don’t think there is any technical disagreement here, not even from > John, Sanjoy, > or Nuno. This is a no-brainer. > > John and I do not have a technical disagreement, John is having an > emotional > reaction to the fact that he doesn’t have an answer to the > function-inlining question. > We can’t disagree until he actually has an opinion to disagree with, but > he is not > willing to express one. > > The work on “poison” and “freeze” should be in new analysis and transform > passes > in which folks can do whatever they want to propagate “undefined behavior” > around > in a function (but “undefined behavior" should be an analysis attribute > not an IR > instruction). Then folks can try to see if they can actually measure a > performance gain > on any SPEC benchmarks. I think we all know this is going to turn out > negative, but that’s > besides the point, the point is that “poison” and “freeze” are an > experiment and need > to be treated as such and not just simply forced into the trunk because > someone likes it. > > What you are saying about ’the llvm way" goes against everything we know > about > "software engineering”, that things have to be evidence based, have > utility, and > be the least complex solution to avoid confusion. And we do engineering > reviews > and take review feedback seriously. I suggest you take a step back and > think about > that, because it sure seems to me like you’re advocating that we don’t do > reviews and > we don’t base decisions on evidence. > > > Peter Lawrence. > > > > On Jun 28, 2017, at 12:16 PM, Chandler Carruth <chandlerc at gmail.com> > wrote: > > On Wed, Jun 28, 2017 at 9:39 AM Peter Lawrence <peterl95124 at sbcglobal.net> > wrote: > >> >> >> Part I. >> >> The original LangRef appeared to be “nice and pretty” >> and originally ‘undef’ did not seem to stick out. >> >> Then evidence came to light in the form of bug reports, and >> in the form of Dan Gohman’s email “the nsw story”, that all >> was not good for ‘undef’ [1,2]. >> >> A proposal was made based on that evidence. >> A presentation was made at an llvm gathering. >> A paper was written. The proposal has even been >> implemented and tested. >> >> The IR might not be so “nice and pretty” anymore, >> but at least all the known bugs could be closed. >> I think everyone agreed that the case could be closed >> based on the evidence at hand. >> >> However new evidence has come to light, >> the function-inlining example for one, >> which the proposal does not address. >> >> This means the case must be re-opened. >> > > Peter, > > People have been continuing to work on these issues for years. This is not > new, and and it is not only now being reopened. > > > Unfortunately, at this point I think you are repeating well known and well > understood information in email after email. I don't think that is a > productive way to discuss this. However, I don't want to dissuade you from > contributing to the project. But I don't think new emails on this subject > will not be a good use of anyone's time. > > Instead, someone needs to go do the very hard work of building, testing, > and understanding solutions to some of these problems. In fact, a few > others are already doing exactly this. > > I understand you disagree with the approach others are taking, and that is > perfectly fine, even good! You have explained your concern, and there > remains a technical disagreement. This is OK. Repeating your position won't > really help move forward. > > Contributing technical perspectives (especially different ones!) is always > valuable, and I don't want to ever discourage it. But when there remains a > strong technical disagreement, we have to find some way to make > progress.Typically, LLVM lends weight towards those who have the most > significant contributions to LLVM in the area *and* are actually doing the > work to realize a path forward. This doesn't make them any more "right" or > their opinions "better". It is just about having a path forward. > > But this should also show you how to make progress. Continuing to debate > in email probably won't help as you're relatively new to the LLVM project. > Instead, write the code, get the data and benchmark results to support your > position and approach, and come back to us. I think you will have to do the > engineering work of building the solution you want (and others disagree > with) and showing why it is objectively better. > > > > _______________________________________________ > LLVM Developers mailing list > llvm-dev at lists.llvm.org > http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev > >-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20170628/4e02f54f/attachment-0001.html>
Chandler,
I am not a “politically correct” person, never have been, never
will be.
If you are waiting for me to make a politically incorrect statement so you can
jump
on it, let me assure you that you will never be disappointed.
But if that’s all you do then you and llvm lose out. If you want to actually
help
llvm move forward then you should judge what I say based on its merit, not on
its
delivery.
The thing I will agree with you about is that people on this list are well
intentioned.
But I still don’t believe my description of “poison” has ever been discussed
before,
you were will intentioned in saying it has, but there is no reason to believe it
has,
because every time I dig deeper into these issues I run into mis-conceptions and
mis-information. Faulty analysis based on faulty assumptions. Every time. If I
sound
hyperbolic it is because I have gotten frustrated from continually running into
this.
And before you claim that I have insulted Dan, you might want to get his
opinion on the subject. My bet is he will react by saying “why didn’t I think
of that”, but in spite of my trying to contact him I have gotten no response.
Perhaps you will have better luck.
Finally, regarding John, when I pushed on the function-inlining example,
rather than responding to it he lashed out at me. That is unprofessional.
I said nothing about it at the time because I try to refrain from
meta-discussions.
But it is another reason why I sound so hyperbolic.
Now, getting back to technical, I’m waiting for some C source code examples
showing how "Current transformations made by LLVM require [posion]”
Peter Lawrence.
> On Jun 28, 2017, at 6:47 PM, Chandler Carruth <chandlerc at
gmail.com> wrote:
>
> On Wed, Jun 28, 2017 at 3:33 PM Peter Lawrence <peterl95124 at
sbcglobal.net <mailto:peterl95124 at sbcglobal.net>> wrote:
> Chandler,
> where we disagree is in whether the current project is
moving the issue
> forward. It is not. It is making the compiler more complex for no
additional value.
>
> I mean, I also disagree with your analysis of where we disagree, and what
is happening, but I don't think that matters much or will convince you of
anything.
>
> The current project perpetuates the myth that “poison” is somehow required.
>
> No one thinks it is required at a theoretical level. Current
transformations made by LLVM require it. We could always disable those
transformations. The current project is attempting to see if there is a
pragmatic set of transformations we can keep with a better definition.
>
> It isn’t, and when I show proof of that you reply with “its in bug reports,
etc”,
> that’s BS and you know it, this hasn’t been explored.
>
> I'm sorry that I didn't have readily available citations in the
other thread. If you can show what searches you have done that didn't find
any results, I'm happy to try and help find them. But the way you say this
doesn't come across as assuming good faith on the part of myself and others
in these discussions. Within the LLVM community, please always assume good faith
and don't accuse people of "BS".
>
> Dan created “poison” on a whim, and people picked up on it too
> without question. We’ve been stuck with this self-inflicted wound ever
since, and it is
> time to heal it.
>
> The way you have described this comes across as both hyperbolic and
insulting to specific individuals.
>
> This kind of behavior and rhetoric is not acceptable in the LLVM community,
and multiple people have asked you to change tone and avoid this behavior.
Bluntly, stop.
>
> John and I do not have a technical disagreement, John is having an
emotional
> reaction to the fact that he doesn’t have an answer to the
function-inlining question.
>
> This is yet another personal attack in your email.
>
> After talking to several people involved in these threads, I think that
whatever technical points you are trying to make have been completely lost due
to the repeated pattern of apparent hostility. Your emails are perceived as
insulting and attacking people which is not an appropriate or professional way
to engage in a technical discussion here.
>
> The attitude and approach you are taking on the list is completely
incompatible with this community and project.
>
> I still encourage you to modify LLVM and implement your approach. It is
open source and easy to fork on GitHub. However, please don't continue
sending emails to LLVM lists until you can do so without repeating this
behavior.
>
> -Chandler
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20170628/646542a8/attachment.html>
Sean,
Many thanks for taking the time to respond. I didn’t make
myself clear, I will try to be brief...
> On Jun 28, 2017, at 7:48 PM, Sean Silva <chisophugis at gmail.com>
wrote:
>
>
>
> On Wed, Jun 28, 2017 at 3:33 PM, Peter Lawrence via llvm-dev <llvm-dev
at lists.llvm.org <mailto:llvm-dev at lists.llvm.org>> wrote:
> Chandler,
> where we disagree is in whether the current project is
moving the issue
> forward. It is not. It is making the compiler more complex for no
additional value.
>
> The current project is not based in evidence, I have asked for any SPEC
benchmark
> that shows performance gain by the compiler taking advantage of “undefined
behavior”
> and no one can show that.
>
> One easy way to measure this is to enable -fwrapv/-ftrapv on the compiler
command line, which impede the compiler's usual exploitation of signed wrap
UB in C. Last I heard, those options lead to substantial performance
regressions.
>
In my other emails I point out that Dan achieved the goal of hoisting
sign-extension
out of loops for LP64 targets by exploiting “+nsw”, and that this appears to be
the only example of a performance benefit. I am not suggesting we remove this
optimization, and my emails point out that keeping this opt does not require
that the
compiler model “undefined behavior”.
Heres what gcc says about those options
-ftrapv
<>This option generates traps for signed overflow on addition,
subtraction, multiplication operations.
-fwrapv
<>This option instructs the compiler to assume that signed arithmetic
overflow of addition, subtraction and multiplication wraps around using
twos-complement representation. This flag enables some optimizations and
disables others. This option is enabled by default for the Java front end, as
required by the Java language specification.
Sounds like -fwrapv has equivocal results, at least for gcc,
my guess is that the same applies to llvm,
If anyone can show that -fwrapv makes a significant drop in SPEC-INT performance
on
a plain old 32-bit machine then we need to look into it because it is kind of
hard to believe
and doesn’t sound consistent with gcc.
I would do the tests myself, but all I have is my Mac-mini which is an LP64
machine,
(and no license for SPEC sources)
On an LP64 machine we would need a flag to disable all but Dan’s optimization to
to do a comparison. It would take some time, but it is doable.
Well, I guess that wasn’t brief, sorry!
Peter Lawrence.
> It sounds like you are implicitly claiming that there would be no
performance regression. If the -fwrapv/-ftrapv measurements are in line with
this, that will be useful new information for the discussion. But I think that
you need to do the work here (and it isn't that much work compared to
writing out emails; or perhaps I'm just a slow email writer).
>
> I don't think anybody has really up to date numbers on the performance
effect of those options on SPEC. Could you please get some up to date numbers
before continuing this discussion? If the results are as you seem to imply, then
that will be very convincing support for your point.
>
> If you can't be bothered to do that, I have to question how invested
you are in pushing the LLVM project toward a better solution to its current woes
with the poison/undef situation.
>
> -- Sean Silva
>
>
> The current project doesn’t even address some of the bugs described in the
paper,
> in particular those derived from the incorrect definition of “undef” in the
LangRef.
>
> The current project perpetuates the myth that “poison” is somehow required.
> It isn’t, and when I show proof of that you reply with “its in bug reports,
etc”,
> that’s BS and you know it, this hasn’t been explored. The same thing
happened
> before when I said “nsw” shouldn’t be hoisted, folks replied with “that’s
already
> been studied and rejected”, but I did the research and found out no, it had
not
> been fully explored, Dan discarded the idea based on incorrect analysis and
people
> never questioned it. Dan created “poison” on a whim, and people picked up
on it too
> without question. We’ve been stuck with this self-inflicted wound ever
since, and it is
> time to heal it.
>
> The entire situation here can be summarized as incorrect analysis, and
failure to
> fully root-cause problems, because people didn’t question their
assumptions.
> And we should not be surprised, it is one of the most common problems in
software
> engineering. Haven’t you ever gone in to fix a bug only to find that what
you are doing
> is correcting someone else’s bug fix that didn’t actually fix anything
because the person
> doing the fix didn’t fully understand the problem? Happens to me all the
time.
>
> The correct software engineering decision here is to fix the definition of
“undef”,
> delete “poison”, and not hoist “nsw” attributes. That is a no-brainer.
There is nothing
> to try out, or test, ormeasure. That is simply the way it has to be to
avoid the current
> set of problems.
>
> I cannot emphasize that last point enough, fixing the definition of
“undef”, deleting
> “poison”, and not allowing “nsw” attributes to be hoisted, fixes all known
problems,
> even including ones that weren’t thought of before these discussions
started, and
> I don’t think there is any technical disagreement here, not even from John,
Sanjoy,
> or Nuno. This is a no-brainer.
>
> John and I do not have a technical disagreement, John is having an
emotional
> reaction to the fact that he doesn’t have an answer to the
function-inlining question.
> We can’t disagree until he actually has an opinion to disagree with, but he
is not
> willing to express one.
>
> The work on “poison” and “freeze” should be in new analysis and transform
passes
> in which folks can do whatever they want to propagate “undefined behavior”
around
> in a function (but “undefined behavior" should be an analysis
attribute not an IR
> instruction). Then folks can try to see if they can actually measure a
performance gain
> on any SPEC benchmarks. I think we all know this is going to turn out
negative, but that’s
> besides the point, the point is that “poison” and “freeze” are an
experiment and need
> to be treated as such and not just simply forced into the trunk because
someone likes it.
>
> What you are saying about ’the llvm way" goes against everything we
know about
> "software engineering”, that things have to be evidence based, have
utility, and
> be the least complex solution to avoid confusion. And we do engineering
reviews
> and take review feedback seriously. I suggest you take a step back and
think about
> that, because it sure seems to me like you’re advocating that we don’t do
reviews and
> we don’t base decisions on evidence.
>
>
> Peter Lawrence.
>
>
>> On Jun 28, 2017, at 12:16 PM, Chandler Carruth <chandlerc at
gmail.com <mailto:chandlerc at gmail.com>> wrote:
>>
>> On Wed, Jun 28, 2017 at 9:39 AM Peter Lawrence <peterl95124 at
sbcglobal.net <mailto:peterl95124 at sbcglobal.net>> wrote:
>>
>>
>> Part I.
>>
>> The original LangRef appeared to be “nice and pretty”
>> and originally ‘undef’ did not seem to stick out.
>>
>> Then evidence came to light in the form of bug reports, and
>> in the form of Dan Gohman’s email “the nsw story”, that all
>> was not good for ‘undef’ [1,2].
>>
>> A proposal was made based on that evidence.
>> A presentation was made at an llvm gathering.
>> A paper was written. The proposal has even been
>> implemented and tested.
>>
>> The IR might not be so “nice and pretty” anymore,
>> but at least all the known bugs could be closed.
>> I think everyone agreed that the case could be closed
>> based on the evidence at hand.
>>
>> However new evidence has come to light,
>> the function-inlining example for one,
>> which the proposal does not address.
>>
>> This means the case must be re-opened.
>>
>> Peter,
>>
>> People have been continuing to work on these issues for years. This is
not new, and and it is not only now being reopened.
>>
>>
>> Unfortunately, at this point I think you are repeating well known and
well understood information in email after email. I don't think that is a
productive way to discuss this. However, I don't want to dissuade you from
contributing to the project. But I don't think new emails on this subject
will not be a good use of anyone's time.
>>
>> Instead, someone needs to go do the very hard work of building,
testing, and understanding solutions to some of these problems. In fact, a few
others are already doing exactly this.
>>
>> I understand you disagree with the approach others are taking, and that
is perfectly fine, even good! You have explained your concern, and there remains
a technical disagreement. This is OK. Repeating your position won't really
help move forward.
>>
>> Contributing technical perspectives (especially different ones!) is
always valuable, and I don't want to ever discourage it. But when there
remains a strong technical disagreement, we have to find some way to make
progress.Typically, LLVM lends weight towards those who have the most
significant contributions to LLVM in the area *and* are actually doing the work
to realize a path forward. This doesn't make them any more "right"
or their opinions "better". It is just about having a path forward.
>>
>> But this should also show you how to make progress. Continuing to
debate in email probably won't help as you're relatively new to the LLVM
project. Instead, write the code, get the data and benchmark results to support
your position and approach, and come back to us. I think you will have to do the
engineering work of building the solution you want (and others disagree with)
and showing why it is objectively better.
>
>
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org <mailto:llvm-dev at lists.llvm.org>
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
<http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20170628/d4a5f928/attachment-0001.html>
On 06/28/2017 05:33 PM, Peter Lawrence wrote:> Chandler, > where we disagree is in whether the current project is > moving the issue > forward. It is not. It is making the compiler more complex for no > additional value. > > The current project is not based in evidence, I have asked for any > SPEC benchmark > that shows performance gain by the compiler taking advantage of > “undefined behavior” > and no one can show that.I can't comment on SPEC, but this does remind me of code I was working on recently. To abstract the relevant parts, it looked something like this: template <typename T> int do_something(T mask, bool cond) { if (mask & 2) return 1; if (cond) { T high_mask = mask >> 48; if (high_mask > 5) do_something_1(high_mask); else if (high_mask > 3) do_something_2(); } return 0; } This function ended up being instantiated on different types T (e.g. unsigned char, unsigned int, unsigned long, etc.) and, dynamically, cond was always false when T was char. The question is: Can the compiler eliminate all of the code predicated on cond for the smaller types? In this case, this code was hot, and moreover, performance depended on the fact that, for T = unsigned char, the function was inlined and the branch on cond was eliminated. In the relevant translation unit, however, the compiler would never see how cond was set. Luckily, we do the right thing here currently. In the case where T = unsigned char, we end up folding both of the high_mask tests as though they were false. That entire part of the code is eliminated, the function is inlined, and everyone is happy. Why was I looking at this? As it turns out, if the 'else if' in this example is just 'else', we don't actually eliminate both sides of the branch. The same is true for many other variants of the conditionals (i.e. we don't recognize all of the code as dead). Once we have a self-consistent model for undef, we should be able to fix that. The user was confused, however, why seemingly innocuous changes to the code changed the performance characteristics of their application. The proposed semantics by John, et al. should fix this uniformly. In any case, to your point about:> if (a == a) > S;I have the same thought. If a == undef here, the code should be dead. Dead code must be aggressively dropped to enable inlining and further optimization. This is an important way we eliminate abstraction penalties. Dead code also has costs in terms of register allocation, speculative execution, inlining, etc. I've also seen cases where templated types are used with fixed-sized arrays where the compiler to leveraged knowledge of UB on uninitialized values and out-of-bounds accesses to eliminate unnecessary part of the code. In short, "optimizing on undefined behavior" can end up being an important tool. -Hal -- Hal Finkel Lead, Compiler Technology and Programming Languages Leadership Computing Facility Argonne National Laboratory -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20170629/31ffcd12/attachment.html>
> On Jun 29, 2017, at 4:39 AM, Hal Finkel <hfinkel at anl.gov> wrote: > > On 06/28/2017 05:33 PM, Peter Lawrence wrote: >> Chandler, >> where we disagree is in whether the current project is moving the issue >> forward. It is not. It is making the compiler more complex for no additional value. >> >> The current project is not based in evidence, I have asked for any SPEC benchmark >> that shows performance gain by the compiler taking advantage of “undefined behavior” >> and no one can show that. > > I can't comment on SPEC, but this does remind me of code I was working on recently. To abstract the relevant parts, it looked something like this: > > template <typename T> > int do_something(T mask, bool cond) { > if (mask & 2) > return 1; > > if (cond) { > T high_mask = mask >> 48; > if (high_mask > 5) > do_something_1(high_mask); > else if (high_mask > 3) > do_something_2(); > } > > return 0; > } > > This function ended up being instantiated on different types T (e.g. unsigned char, unsigned int, unsigned long, etc.) and, dynamically, cond was always false when T was char. The question is: Can the compiler eliminate all of the code predicated on cond for the smaller types? In this case, this code was hot, and moreover, performance depended on the fact that, for T = unsigned char, the function was inlined and the branch on cond was eliminated. In the relevant translation unit, however, the compiler would never see how cond was set. > > Luckily, we do the right thing here currently. In the case where T = unsigned char, we end up folding both of the high_mask tests as though they were false. That entire part of the code is eliminated, the function is inlined, and everyone is happy. > > Why was I looking at this? As it turns out, if the 'else if' in this example is just 'else', we don't actually eliminate both sides of the branch. The same is true for many other variants of the conditionals (i.e. we don't recognize all of the code as dead).I apologize in advance if I have missed something here and am misreading your example... This doesn’t make sense to me, a shift amount of 48 is “undefined” for unsigned char, How do we know this isn’t a source code bug, What makes us think the the user intended the result to be “0”. This strikes me as odd, we are mis-interpreting the user’s code In such a way so as to improve performance, but that isn’t necessarily what the user intended. Here’s one way to look at this issue, if something is “C undefined behavior” then The standard says, among other things, that we could trap here Why aren’t we doing that rather than optimizing it ? Here’s another way to look at it, no one has ever filed a bug that reads “I used undefined behavior in my program, but the optimizer isn’t taking advantage of it” But if they do I think the response should be “you should not expect that, standard says nothing positive about what undefined behavior does"> Once we have a self-consistent model for undef, we should be able to fix that. The user was confused, however, why seemingly innocuous changes to the code changed the performance characteristics of their application. The proposed semantics by John, et al. should fix this uniformly. > > In any case, to your point about: > >> if (a == a) >> S; > > I have the same thought. If a == undef here, the code should be dead. Dead code must be aggressively dropped to enable inlining and further optimization. This is an important way we eliminate abstraction penalties. Dead code also has costs in terms of register allocation, speculative execution, inlining, etc. >And yet IIRC Sanjoy in his last email was arguing for consistent behavior in cases like If (x != 0) { /* we can optimize in the then-clause assuming x != 0 */ } And in the case above when it is a function that gets inlined Here’s what Sanjoy said about the function-inline case> This too is fixed in the semantics mentioned in the paper. This also > isn't new to us, it is covered in section 3.1 "Duplicate SSA Uses".So this issue seems to be up in the air> I've also seen cases where templated types are used with fixed-sized arrays where the compiler to leveraged knowledge of UB on uninitialized values and out-of-bounds accesses to eliminate unnecessary part of the code. In short, "optimizing on undefined behavior" can end up being an important tool. >As you can tell from my first comments, I am not yet convinced, and would still like to see real evidence Peter Lawrence.> -Hal > -- > Hal Finkel > Lead, Compiler Technology and Programming Languages > Leadership Computing Facility > Argonne National Laboratory-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20170629/b742f1c1/attachment.html>