[lld] r288409 - Updates file comments and variable names.

[Sean Silva via llvm-commits]

On Fri, Dec 2, 2016 at 3:52 PM, Rui Ueyama <ruiu at google.com> wrote:

> I'm not familiar with GVN, but even if it is similar to it, I don' think I
> *should* use the same term used there. I originally chose GroupId because
> it was a group id (and the term "group" is used in gold's safe ICF paper to
> describe the concept http://static.googleusercontent.com/media/
> research.google.com/en//pubs/archive/36912.pdf), and then changed it to
> Color because it's one word and easy to digest than GroupId to me (e.g.
> we'll just color them in different colors!). Congruence class is, well, it
> sounds too technical and doesn't improve readability at least to me.
>

Personally, I'm pretty opposed to the term "color" here. In all the cases I
can think of, algorithms that refer to "colors" the number of different
colors is a constant (one talks about "k-colorability" etc.; the "k" is
always a constant). That is not the case in this context and it is really
confusing. This is *not* a coloring algorithm; coloring algorithms are
about local constraints where adjacent things can't be colored the same.

GVN/CSE and the term "congruence" (or even "equivalence class") is
technically correct. GroupId is consistent with the paper at least. But
"coloring" is just wrong.

-- Sean Silva


>
> On Fri, Dec 2, 2016 at 12:46 AM, Sean Silva <chisophugis at gmail.com> wrote:
>
>>
>>
>> On Fri, Dec 2, 2016 at 12:35 AM, Davide Italiano <davide at freebsd.org>
>> wrote:
>>
>>> On Fri, Dec 2, 2016 at 12:34 AM, Sean Silva via llvm-commits
>>> <llvm-commits at lists.llvm.org> wrote:
>>> >
>>> >
>>> > On Thu, Dec 1, 2016 at 11:45 AM, Rui Ueyama via llvm-commits
>>> > <llvm-commits at lists.llvm.org> wrote:
>>> >>
>>> >> Author: ruiu
>>> >> Date: Thu Dec  1 13:45:22 2016
>>> >> New Revision: 288409
>>> >>
>>> >> URL: http://llvm.org/viewvc/llvm-project?rev=288409&view=rev
>>> >> Log:
>>> >> Updates file comments and variable names.
>>> >>
>>> >> Use "color" instead of "group id" to describe the ICF algorithm.
>>> >
>>> >
>>> > The right term is "congruence class"; I think you should use it. This
>>> ICF
>>> > algorithm is basically a simple "optimistic" GVN/CSE algorithm; all
>>> values
>>> > are initially assumed to be in the same congruence class and then that
>>> > equivalence class is iteratively split as contradictions are found
>>> until
>>> > there are no contradictions.
>>> >
>>>
>>> +1, I think the proper term is congruence here.
>>>
>>
>> I think you've been working on NewGVN for long enough to know better than
>> "I think" ;)
>>
>> -- Sean Silva
>>
>>
>>>
>>> --
>>> Davide
>>>
>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20161202/8511f6c0/attachment.html>