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

[Sean Silva via llvm-commits]

On Sun, Dec 4, 2016 at 9:18 AM, Rui Ueyama <ruiu at google.com> wrote:

> On Fri, Dec 2, 2016 at 9:24 PM, Sean Silva <chisophugis at gmail.com> wrote:
>
>>
>>
>> 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.googleuserconten
>>> t.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.
>>
>
> If you feel strongly about that, I'm fine to change the term, but how do
> you describe it?
>
> I think congruence class is defined as this: if two sections are in the
> same congruence class, they are identical.
>
> That refers the result of the algorithm. We cannot say that we split
> congruence class into smaller congruence classes on each iteration, because
> if a congruence class can be split, it wasn't a congruence class.
>

Not quite. You can think about it like each iteration makes the congruence
relation more precise, until "section A is congruent to section B" means
"section A is identical to B". That is why the term "congruent" is used
instead of "identical".
For an optimistic algorithm, we initially consider all sections congruent.
Then we apply a series of refinements to the congruence relation until we
know that if two sections are congruent, then they are identical.

The sense of the term "congruent" is similar to this sense in math:
http://mathworld.wolfram.com/Congruence.html
I.e. "congruent" means "identical in this regard", and there is a special
symbol (the equal sign with three lines) to represent this, to distinguish
it from truly "identical".


I agree with you that congruence is fairly technical terminology.
"equivalence class" means the same thing and is easier to understand.

I think the description we already had is quite good:

```
-// We begin by optimistically putting all sections into a single
equivalence
-// class. Then we apply a series of checks that split this initial
-// equivalence class into more and more refined equivalence classes based
on
-// the properties by which a section can be distinguished.
-//
-// We begin by checking that the section contents and flags are the
-// same. This only needs to be done once since these properties don't
depend
-// on the current equivalence class assignment.
-//
-// Then we split the equivalence classes based on checking that their
-// relocations are the same, where relocation targets are compared by their
-// equivalence class, not the concrete section. This may need to be done
-// multiple times because as the equivalence classes are refined, two
-// sections that had a relocation target in the same equivalence class may
-// now target different equivalence classes, and hence these two sections
-// must be put in different equivalence classes (whereas in the previous
-// iteration they were not since the relocation target was the same.)
```


Mostly, I just really object to the use of the term "color" because it is
really confusing in this context.


-- Sean Silva


>
> 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/20161204/66ff0d12/attachment.html>