This is an archive of the discontinued LLVM Phabricator instance.

[InstCombine] extractvalue (any_mul_with_overflow X, 2^n), 0 -> X << n
ClosedPublic

Authored by Chenbing.Zheng on Sep 2 2022, 12:04 AM.

Details

Summary

Alive2: https://alive2.llvm.org/ce/z/JLmabt (umul)

https://alive2.llvm.org/ce/z/J_ruXR  (smul)
https://alive2.llvm.org/ce/z/o9SVSz (vector)

Diff Detail

Event Timeline

Chenbing.Zheng created this revision.Sep 2 2022, 12:04 AM
Herald added a project: Restricted Project. · View Herald TranscriptSep 2 2022, 12:04 AM
Herald added a subscriber: hiraditya. · View Herald Transcript
Chenbing.Zheng requested review of this revision.Sep 2 2022, 12:04 AM
Chenbing.Zheng edited the summary of this revision. (Show Details)Sep 2 2022, 12:10 AM
spatel added a comment.Sep 2 2022, 9:03 AM

Why not do the transform with smul too?

RKSimon added a comment.EditedSep 2 2022, 9:35 AM

Is this a better proof? https://alive2.llvm.org/ce/z/JLmabt (noundefs) https://alive2.llvm.org/ce/z/J_ruXR (with undefs)

Chenbing.Zheng retitled this revision from [InstCombine] extractvalue (umul_with_overflow X, 2^n), 0 -> X << n to [InstCombine] extractvalue (any_mul_with_overflow X, 2^n), 0 -> X << n.
Chenbing.Zheng edited the summary of this revision. (Show Details)

add smul support

Is this a better proof? https://alive2.llvm.org/ce/z/JLmabt (noundefs) https://alive2.llvm.org/ce/z/J_ruXR (with undefs)

Yer, thanks. I have update it to summary.

spatel added inline comments.Sep 5 2022, 5:43 AM
llvm/test/Transforms/InstCombine/with_overflow.ll
1007

Wrong comment or wrong test?

Chenbing.Zheng marked an inline comment as done.Sep 5 2022, 6:53 PM
spatel added a comment.Sep 6 2022, 5:18 AM

This change allows vectors with undef elements, but there are no tests for that. Please adjust at least one test to exercise that pattern - a vector type with constant that includes a poison element.
You can also verify a specific constant with poison with Alive2 and add a link to the patch summary.

RKSimon added inline comments.Sep 6 2022, 5:28 AM
llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
3285–3286

m_AllOnes ?

Chenbing.Zheng edited the summary of this revision. (Show Details)

add vector tests

Chenbing.Zheng added inline comments.Sep 7 2022, 1:33 AM
llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
3285–3286

We don't just deal with AllOnes

spatel accepted this revision.Sep 7 2022, 5:10 AM

LGTM

This revision is now accepted and ready to land.Sep 7 2022, 5:10 AM
This revision was landed with ongoing or failed builds.Sep 7 2022, 8:15 PM
This revision was automatically updated to reflect the committed changes.