This is an archive of the discontinued LLVM Phabricator instance.

[InstSimplify] Simplify smul.fix and smul.fix.sat
ClosedPublic

Authored by bjope on Mar 9 2021, 3:15 PM.

Details

Summary

Add simplification of smul.fix and smul.fix.sat according to

X * 0 -> 0
X * undef -> 0
X * (1 << scale) -> X

This includes the commuted patterns and splatted vectors.

Diff Detail

Event Timeline

bjope created this revision.Mar 9 2021, 3:15 PM
bjope requested review of this revision.Mar 9 2021, 3:15 PM
Herald added a project: Restricted Project. · View Herald TranscriptMar 9 2021, 3:15 PM
nikic accepted this revision.Mar 10 2021, 12:13 PM

LGTM

This revision is now accepted and ready to land.Mar 10 2021, 12:13 PM
nagisa added a subscriber: nagisa.Mar 10 2021, 3:44 PM
nagisa added inline comments.
llvm/lib/Analysis/InstructionSimplify.cpp
5767

Wouldn't this fail to trigger if the intrinsic call starts as follows?

%res = call i16 @llvm.smul.fix.i16(i16 42, i16 undef, i32 ...)

If I understand it correctly, the canonicalization step above would swap the two operands, making the Op0 = undef and Op1 = 42, thus preventing this simplification from happening.

(This does not appear to be an issue for 0, it seems there's code somewhere else that const-evaluates this operation if both operands are constant)

nikic added inline comments.Mar 11 2021, 1:23 AM
llvm/lib/Analysis/InstructionSimplify.cpp
5767

Folding a call with all constant arguments is the responsibility of ConstantFolding rather than InstSimplify. Possibly the current constant folding implementation for fix-point multiplication doesn't handle undefs though.

bjope added inline comments.Mar 11 2021, 1:44 AM
llvm/lib/Analysis/InstructionSimplify.cpp
5767

@nagisa : Nice finding!
@nikic : Yes, that was a limitation in ConstantFolding. I've already started to prepare a fix for that but need to create some more tests. I can make that a predecessor for this patch (or maybe I'll just squash them together).

bjope updated this revision to Diff 329914.Mar 11 2021, 4:01 AM

Update code comments (those comments depends on landing D98410 first).

This revision was automatically updated to reflect the committed changes.