[PATCH] D26224: NewGVN

[Daniel Berlin via llvm-commits]

>
>
>
> ================
> Comment at: lib/Transforms/Scalar/NewGVN.cpp:1455
> +
> +    if (DFSIn < other.DFSIn)
> +      return true;
> ----------------
> This is just implementing a lexicographical comparison, right?
>
Yes

>
> If so, I would do:
>
> ```
> return std::tie(DFSIn, DFSOut, ...) < std::tie(other.DFSIn, other.DFSOut,
> ...);
>


When i started this code, we didn't have tuple (it's that old!), but yeah,
this is what should be done now.


> ```
>
>
> ================
> Comment at: lib/Transforms/Scalar/NewGVN.cpp:1464
> +        else if (LocalNum == other.LocalNum) {
> +          if (Val < other.Val)
> +            return true;
> ----------------
> This seems super sketchy, comparing < on pointers. What's up with that?

 It's really just here for completeness,but agree it looks sketchy.


> Won't that make the result nondeterministic?

No, actually, though i admit it's not obvious to see this.
 It's not possible to push or pop a value for elimination differently  in a
way that matters based on the .val (and you should be able to prove this).
  Each LLVM instruction only produces one value, and thus  the lowest-level
differentiator that really matters for the stack (and what we use as as a
replacement) is the local dfs number.
Everything else in the structure is instruction level, and only affects the
order in which we will replace operands of a given instruction.

For a given instruction (IE things with equal dfsin, dfsout, localnum), the
order of replacement of uses does not matter (and no other pass guarantees
it, AFAIK)

IE given,

a = 5
b = a + a

When you hit b, you will have two valuedfs with the same dfsin, out, and
localnum.
The .val will be the same as well.
The .u's will be different.

You will replace both, and it does not matter what order you replace them
in (IE whether you replace operand 2, then operand 1, or operand 1, then
operand 2).


Similarly for the case of same dfsin, dfsout, localnum, but different
.val's

a = 5
b  = 6
c = a + b

in c, we will a valuedfs for a, and one for b,with everything the same but
.val  and .u.
It does not matter what order we replace these operands in.

You will always end up with the same IR, and this is guaranteed.



>
> https://reviews.llvm.org/D26224
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20161213/60f095d6/attachment.html>