[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 cfe-commits
cfe-commits at lists.llvm.org
Sun Oct 4 10:28:16 PDT 2020
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/cfe-commits/attachments/20201004/56b03fb9/attachment-0001.bin>
More information about the cfe-commits
mailing list