[all-commits] [llvm/llvm-project] e00f18: [InstCombine] Revert rL226781 "Teach InstCombine t...

[Roman Lebedev via All-commits]

  Branch: refs/heads/master
  Home:   https://github.com/llvm/llvm-project
  Commit: e00f189d392dd9bf95f6a98f05f2d341d06cd65c
      https://github.com/llvm/llvm-project/commit/e00f189d392dd9bf95f6a98f05f2d341d06cd65c
  Author: Roman Lebedev <lebedev.ri at gmail.com>
  Date:   2020-10-06 (Tue, 06 Oct 2020)

  Changed paths:
    M clang/test/CodeGen/attr-arm-sve-vector-bits-bitcast.c
    M clang/test/CodeGen/attr-arm-sve-vector-bits-call.c
    M clang/test/CodeGen/attr-arm-sve-vector-bits-cast.c
    M clang/test/CodeGen/attr-arm-sve-vector-bits-globals.c
    M llvm/lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp
    M llvm/test/Transforms/InstCombine/atomic.ll
    M llvm/test/Transforms/InstCombine/load.ll
    M llvm/test/Transforms/InstCombine/loadstore-metadata.ll
    M llvm/test/Transforms/InstCombine/non-integral-pointers.ll
    M llvm/test/Transforms/PhaseOrdering/instcombine-sroa-inttoptr.ll

  Log Message:
  -----------
  [InstCombine] Revert rL226781 "Teach InstCombine to canonicalize loads which are only ever stored to always use a legal integer type if one is available." (PR47592)

(it was introduced in https://lists.llvm.org/pipermail/llvm-dev/2015-January/080956.html)

This canonicalization seems dubious.

Most importantly, while it does not create `inttoptr` casts by itself,
it may cause them to appear later, see e.g. D88788.

I think it's pretty obvious that it is an undesirable outcome,
by now we've established that seemingly no-op `inttoptr`/`ptrtoint` casts
are not no-op, and are no longer eager to look past them.
Which e.g. means that given
```
%a = load i32
%b = inttoptr %a
%c = inttoptr %a
```
we likely won't be able to tell that `%b` and `%c` is the same thing.

As we can see in D88789 / D88788 / D88806 / D75505,
we can't really teach SCEV about this (not without the https://bugs.llvm.org/show_bug.cgi?id=47592 at least)
And we can't recover the situation post-inlining in instcombine.

So it really does look like this fold is actively breaking
otherwise-good IR, in a way that is not recoverable.
And that means, this fold isn't helpful in exposing the passes
that are otherwise unaware of these patterns it produces.

Thusly, i propose to simply not perform such a canonicalization.
The original motivational RFC does not state what larger problem
that canonicalization was trying to solve, so i'm not sure
how this plays out in the larger picture.

On vanilla llvm test-suite + RawSpeed, this results in
increase of asm instructions and final object size by ~+0.05%
decreases final count of bitcasts by -4.79% (-28990),
ptrtoint casts by -15.41% (-3423),
and of inttoptr casts by -25.59% (-6919, *sic*).
Overall, there's -0.04% less IR blocks, -0.39% instructions.

See https://bugs.llvm.org/show_bug.cgi?id=47592

Differential Revision: https://reviews.llvm.org/D88789