Instcombine and instsimplify have similar optimizations , may I copy optmization from instcombine to instsimplify?

Beginners
1

hello!

I’ll try to create patch to fix TODO in llvm/test/Transforms/InstSimplify/addsub.ll
like below

; TODO: simplify this to %a
define i1 @test8(i1 %a) {
; CHECK-LABEL: @test8(
; CHECK-NEXT: [[C:%.]] = add i1 [[A:%.]], true
; CHECK-NEXT: [[RES:%.*]] = xor i1 [[C]], true
; CHECK-NEXT: ret i1 [[RES]]
;
%c = add i1 %a, true
%res = xor i1 %c, true
ret i1 %res
}

this test cases have TODO comments.
add & xor has the same truth table if i1.
therefore add can be replaced with xor
so, test7 could be optimized.

here is test case

define i1 @test7(i1 %a) {
; CHECK-LABEL: @test7(
; CHECK-NEXT: ret i1 [[A:%.*]]
;
%c = xor i1 %a, true
%res = add i1 %c, true
ret i1 %res
}

and this is code

/// i1 add → xor.
if (MaxRecurse && Op0->getType()->isIntOrIntVectorTy(1))
if (Value *V = simplifyXorInst(Op0, Op1, Q, MaxRecurse - 1))
return V;

as you can see, it work for test7 but not enough test8.
but instcombine can do it. and this is code how instcombine pass optimize test8.

Value *LHS = I.getOperand(0), *RHS = I.getOperand(1);
Type *Ty = I.getType();
if (Ty->isIntOrIntVectorTy(1))
return BinaryOperator::CreateXor(LHS, RHS);

I have two questions. Why didn’t instsimplify apply the logic of instcombine as it is?
And can I use the logic of instcombine for optimize test8 in instsimplify?

thanks!

2

InstCombine uses InstSimplify, so InstCombine is a superset of InstSimplify. The difference is InstSimplify doesn’t modify or introduce instructions. It only performs simplifications where it can return an existing value from its operands or a constant.

3

understand.

May I ask a question about test8, the TODO mentioned above?

add/xor/sub has the same truth table for i8 only.
therefore xor in %res = xor i1 %c, true can replaced with add like this %res = add i1 %c, true

This change makes it associative, so only %a is returned.
Is this allowed?

@arsenm

4

Yes, if you can find %a without modifying any other instructions it’s fine

1 Like
5

This comes with the caveat that just because InstSimplify is allowed to do something, doesn’t mean that it should.

In particular, add i1 %a, true is not canonical IR, so InstSimplify should not go out of the way to handle it without specific motivation.

1 Like
6

I’m not sure if I understood correctly.

I look optimization logic that specific to the case where the operand is i1 in instsimplify. There are implementations for simplifyAddInst, simplifySubInst, simplifyXorInst, etc., among which:

For example, simplifyAddInst calls simplifyXorInst given an operand of type i1.

/// i1 add → xor.
if (MaxRecurse && Op0->getType()->isIntOrIntVectorTy(1))
if (Value *V = simplifyXorInst(Op0, Op1, Q, MaxRecurse - 1))
return V;

I want to add a transformation like this.
I’m wondering if this logic don’t need like you said.

For example:

/// i1 add, sub, xor can be switched.
if (MaxRecurse && Op0->getType()->isIntOrIntVectorTy(1)) {
BinaryOperator* Op0BinOp = dyn_cast(Op0);
if (Op0BinOp) {
auto Op0Opcode = Op0BinOp->getOpcode();
if (Op0Opcode == Instruction::Add) {
if (Value *V = simplifyBinOp(Op0Opcode, Op0, Op1, Q, MaxRecurse))
return V;
}
}
}

7

Yes, this is unnecessary.

If InstCombine already handles a pattern, there is (usually) no need to handle it in InstSimplify unless you can actually move the whole fold from InstCombine to InstSimplify.

1 Like
8

ok, I understand well thanks!