[PATCH] D126040: [InstCombine] Fold a mul with bool value into and
Sanjay Patel via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Tue May 24 07:46:12 PDT 2022
spatel added a comment.
Given the existing folds near here, I'm fine with the small and direct match. But let's make sure others agree with the direction.
================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineMulDivRem.cpp:305
/// i1 mul -> i1 and.
if (I.getType()->isIntOrIntVectorTy(1))
return BinaryOperator::CreateAnd(Op0, Op1);
----------------
I think it would be better to add the new match to this clause since they are folding to the same new value:
if (type is i1 || (match...))
return new and
================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineMulDivRem.cpp:378-381
+ // X * Y --> X & Y, iff X, Y can be only 1. Don't use known bits API as
+ // somewhat unclear from a canonicalization perspective, which may increase
+ // instruction count. (We'd probably want to represent it as trunc(X) ? Y : 0
+ // rather than -X & Y, but it's increasing the count either way.)
----------------
IIUC, the comment about trunc + select was about the case when only one operand has range {0,1}.
So I'd remove that and possibly just add something like:
// Note: We could use known bits to generalize this and related patterns with shifts/truncs.
================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineMulDivRem.cpp:382-383
+ // rather than -X & Y, but it's increasing the count either way.)
+ if (match(Op0, m_And(m_Value(X), m_One())) &&
+ match(Op1, m_And(m_Value(Y), m_One())))
+ return BinaryOperator::CreateAnd(Op0, Op1);
----------------
Don't capture X and Y if we are not using them in the new instruction.
================
Comment at: llvm/test/Transforms/InstCombine/mul-bool.ll:1-2
+; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
+; RUN: opt < %s -passes=instcombine -S | FileCheck %s
+
----------------
It would be better to pre-commit these tests with baseline results in the same file as one of the related transforms. Add to "mul-masked-bits.ll" ?
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D126040/new/
https://reviews.llvm.org/D126040
More information about the llvm-commits
mailing list