Page MenuHomePhabricator

[InstCombine] extract(u[add|sub].with.overflow(X, C), 0) --> [add|sub](X, C)
AbandonedPublic

Authored by nico-abram on Apr 30 2022, 4:15 PM.

Details

Summary

Trying to fix https://github.com/llvm/llvm-project/issues/53978 I ran into something like this: https://alive2.llvm.org/ce/z/bduF3e (generalized here https://alive2.llvm.org/ce/z/HdrUXt ) which is not currently optimized.

@nikic mentioned on discord that it might make more sense to canonicalize the intrinsic into a simple add/sub instead of trying to match that specific pattern

Yes, InstCombine would be the place. Though frankly, we should probably just canonicalize uadd/usub away from intrinsics. That should result in better mid-end optimization, and CGP can reform them

This patch does that, but only when the addition is by a constant. I added @uadd_res_ugt_smaller_const_or_ov and @uadd_res_ult_const_five_and_ov as a baseline (The baseline is in the second diff. The third one is the patch against main HEAD).

With this patch, the original case in https://github.com/llvm/llvm-project/issues/53978 gets optimized to a constant false.

Diff Detail

Event Timeline

nico-abram created this revision.Apr 30 2022, 4:15 PM
Herald added a project: Restricted Project. · View Herald TranscriptApr 30 2022, 4:15 PM
Herald added a subscriber: hiraditya. · View Herald Transcript
nico-abram requested review of this revision.Apr 30 2022, 4:15 PM
Herald added a project: Restricted Project. · View Herald TranscriptApr 30 2022, 4:15 PM
nico-abram updated this revision to Diff 426271.
nico-abram edited the summary of this revision. (Show Details)
nikic added a reviewer: spatel.May 1 2022, 1:15 AM

Hrm, unfortunately canonicalizing the intrinsic to add/icmp is not safe with undef: https://alive2.llvm.org/ce/z/cgiQ_n We effectively introduce two uses of undef, which can each take a different value.

This might need to be addressed on the front-end side instead, by not emitting uadd/usub intrinsics in the first place.

ojeda added a subscriber: ojeda.May 1 2022, 1:36 AM
nico-abram abandoned this revision.May 2 2022, 5:56 PM

Abandoning revision since the transformation is not safe.

"front-end" means rustc/clang right?