[IR][GVN] allow intrinsics in Instruction's isCommutative query

As discussed in D86798 / rG09652721 , we were potentially
returning a different result for whether an Instruction
is commutable depending on if we call the base class or
derived class method.

This requires relaxing an assert in GVN, but that pass
seems to be working otherwise.

NewGVN requires more work because it uses different
code paths for numbering binops and calls.
This commit is contained in:
Sanjay Patel 2020-08-30 15:32:45 -04:00
parent c84adaaae2
commit d58c2f282d
5 changed files with 13 additions and 4 deletions

View File

@ -532,7 +532,7 @@ public:
/// In LLVM, these are the commutative operators, plus SetEQ and SetNE, when
/// applied to any type.
///
bool isCommutative() const { return isCommutative(getOpcode()); }
bool isCommutative() const LLVM_READONLY;
static bool isCommutative(unsigned Opcode) {
switch (Opcode) {
case Add: case FAdd:

View File

@ -673,6 +673,13 @@ bool Instruction::isAssociative() const {
}
}
bool Instruction::isCommutative() const {
if (auto *II = dyn_cast<IntrinsicInst>(this))
return II->isCommutative();
// TODO: Should allow icmp/fcmp?
return isCommutative(getOpcode());
}
unsigned Instruction::getNumSuccessors() const {
switch (getOpcode()) {
#define HANDLE_TERM_INST(N, OPC, CLASS) \

View File

@ -293,7 +293,9 @@ GVN::Expression GVN::ValueTable::createExpr(Instruction *I) {
// of their operands get the same value number by sorting the operand value
// numbers. Since all commutative instructions have two operands it is more
// efficient to sort by hand rather than using, say, std::sort.
assert(I->getNumOperands() == 2 && "Unsupported commutative instruction!");
assert(((isa<BinaryOperator>(I) && I->getNumOperands() == 2) ||
(isa<IntrinsicInst>(I) && I->getNumOperands() == 3))
&& "Unsupported commutative instruction!");
if (e.varargs[0] > e.varargs[1])
std::swap(e.varargs[0], e.varargs[1]);
e.commutative = true;

View File

@ -1254,6 +1254,7 @@ const UnknownExpression *NewGVN::createUnknownExpression(Instruction *I) const {
const CallExpression *
NewGVN::createCallExpression(CallInst *CI, const MemoryAccess *MA) const {
// FIXME: Add operand bundles for calls.
// FIXME: Allow commutative matching for intrinsics.
auto *E =
new (ExpressionAllocator) CallExpression(CI->getNumOperands(), CI, MA);
setBasicExpressionInfo(CI, E);

View File

@ -32,8 +32,7 @@ define void @cmp(i32 %x, i32 %y) {
define void @intrinsic(i32 %x, i32 %y) {
; CHECK-LABEL: @intrinsic(
; CHECK-NEXT: [[M1:%.*]] = call i32 @llvm.umax.i32(i32 [[X:%.*]], i32 [[Y:%.*]])
; CHECK-NEXT: [[M2:%.*]] = call i32 @llvm.umax.i32(i32 [[Y]], i32 [[X]])
; CHECK-NEXT: call void @use(i32 [[M1]], i32 [[M2]])
; CHECK-NEXT: call void @use(i32 [[M1]], i32 [[M1]])
; CHECK-NEXT: ret void
;
%m1 = call i32 @llvm.umax.i32(i32 %x, i32 %y)