[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 created this revision.
lebedev.ri added reviewers: jfb, spatel, efriedma, MaskRay, chandlerc, nlopes.
lebedev.ri added a project: LLVM.
Herald added subscribers: dexonsmith, hiraditya.
lebedev.ri requested review of this revision.

(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 <https://reviews.llvm.org/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.

We could of course try to cleanup the IR afterwards, by enhancing
the `cast-of-load` transform to deal with non-single-use loads,
and i even tried that already in D75505 <https://reviews.llvm.org/D75505>, 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

Thusly, i'd 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.

Does anyone have any thoughts?

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


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D88789

Files:
  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.296001.patch
Type: text/x-patch
Size: 15032 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20201003/04eacc02/attachment.bin>