[PATCH] D98299: [InstSimplify] Simplify smul.fix and smul.fix.sat

[Bjorn Pettersson via Phabricator via llvm-commits]

bjope added inline comments.


================
Comment at: llvm/lib/Analysis/InstructionSimplify.cpp:5765
+    if (Q.isUndefValue(Op1))
+      return Constant::getNullValue(ReturnType);
+
----------------
nikic wrote:
> nagisa wrote:
> > Wouldn't this fail to trigger if the intrinsic call starts as follows?
> > 
> > ```
> > lang=llvm
> > %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)
> 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.
@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).


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D98299/new/

https://reviews.llvm.org/D98299