[PATCH] D88789: [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)

[Roman Lebedev via Phabricator via llvm-commits]

lebedev.ri updated this revision to Diff 296055.
lebedev.ri added a comment.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Rebase/fix remaining tests.

In D88789#2310441 <https://reviews.llvm.org/D88789#2310441>, @nlopes wrote:

> Not introducing inttoptr during optimization is a very healthy goal.

Thank you for pointing that out.
Indeed, that is very precisely my goal here.

In D88789#2310593 <https://reviews.llvm.org/D88789#2310593>, @nikic wrote:

>> as it was rightfully pointed out, that is very much not compile-time free: https://llvm-compile-time-tracker.com/compare.php?from=871d03a6751e0f82e210c80a881ef357c5633a26&to=782be5b99377b62e998e4157ddede0fa296664b5&stat=instructions
>
> Looks free to me?

We can revisit that patch afterwards.

In D88789#2310593 <https://reviews.llvm.org/D88789#2310593>, @nikic wrote:

> In any case, this change looks reasonable to me. GVN has no problems deduplicating load/stores from different types (https://llvm.godbolt.org/z/5nTjWE), so I'm not sure what this canonicalization was useful for.

Yep.

In D88789#2310606 <https://reviews.llvm.org/D88789#2310606>, @chandlerc wrote:

> FWIW, I still very much feel that this is the correct canonicalization, and that downstream problems *must* be fixed downstream. Avoiding this canonicalization doesn't actually fix them, it just makes us less *aware* of the problems that still fundamentally exist. =[
>
> That said, I'm not heavily involved in LLVM, and so if everyone currently involved thinks this is a good change, I'm not going to stand in the way. It just makes no sense to me.

Thank you for commenting!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D88789/new/

https://reviews.llvm.org/D88789

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

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D88789.296055.patch
Type: text/x-patch
Size: 30540 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20201004/56b03fb9/attachment.bin>