[TTI] Consider select form of and/or i1 as having arithmetic cost

[aqjune]

Summary

This is a patch that updates the cost of select i1 a, b, false to be equivalent to that of and i1 a, b
as well as the cost of select i1 a, true, b equivalent to or i1 a, b.

Until now, these selects were folded into and/or i1 by InstCombine, but the transformation is poison-unsafe.
This is a step towards removing the unsafe transformation. D93065 has relevant transformations linked.
These selects should be translated into the assemblies as and/or i1 do in the same manner. The cost should be equivalent.

[aqjune]

This patch has tests missing, and I'd like to add these. Where should I go to do this?

[lebedev.ri]

In D97360#2584048, @aqjune wrote:

This patch has tests missing, and I'd like to add these. Where should I go to do this?

llvm/test/Analysis/CostModel/

[aqjune]

Thanks!
Tests added

[aqjune]

Update tests to test throughput and size both

[aqjune]

Clang-format

[spatel]

I can't tell from the tests (so we should probably add at least 1 to confirm) - do we expect the same behavior for vector types?

llvm/include/llvm/Analysis/TargetTransformInfoImpl.h

997

It would be nice to have a code comment here to show the expected patterns:

// select x, y, false --> x & y
// select x, true, y --> x | y

[aqjune]

Yes, vectors are expected to have the same cost. I'll update tests for them.

[spatel]

In D97360#2594268, @aqjune wrote:

Yes, vectors are expected to have the same cost. I'll update tests for them.

Thanks - IIUC, we're also going to need to some codegen changes to avoid regressions. For example (the icmps are to avoid diffs due to lack of signext/zeroext specifiers on parameters with vector type):

define <4 x i1> @b(<4 x i32> %x, <4 x i32> %y) {
  %c1 = icmp eq <4 x i32> %x, <i32 42, i32 42, i32 42, i32 42>
  %c2 = icmp sgt <4 x i32> %y, <i32 42, i32 42, i32 42, i32 42>
  %s = select <4 x i1> %c1, <4 x i1> <i1 true, i1 true, i1 true, i1 true>, <4 x i1> %c2
  ret <4 x i1> %s
}
define <4 x i1> @b2(<4 x i32> %x, <4 x i32> %y) {
  %c1 = icmp eq <4 x i32> %x, <i32 42, i32 42, i32 42, i32 42>
  %c2 = icmp sgt <4 x i32> %y, <i32 42, i32 42, i32 42, i32 42>
  %s = or <4 x i1> %c1, %c2
  ret <4 x i1> %s
}

$ llc -o - logical.ll -mattr=avx -mtriple=x86_64--
_b:                                     ## @b
	vmovdqa	LCPI2_0(%rip), %xmm2            ## xmm2 = [42,42,42,42]
	vpcmpeqd	%xmm2, %xmm0, %xmm0
	vpcmpgtd	%xmm2, %xmm1, %xmm1
	vblendvps	%xmm0, LCPI2_1(%rip), %xmm1, %xmm0
	retq
_b2:                                    ## @b2
	vmovdqa	LCPI3_0(%rip), %xmm2            ## xmm2 = [42,42,42,42]
	vpcmpeqd	%xmm2, %xmm0, %xmm0
	vpcmpgtd	%xmm2, %xmm1, %xmm1
	vpor	%xmm1, %xmm0, %xmm0
	retq

[aqjune]

add tests for vector operands

[aqjune]

In D97360#2594315, @spatel wrote:

Thanks - IIUC, we're also going to need to some codegen changes to avoid regressions. For example (the icmps are to avoid diffs due to lack of signext/zeroext specifiers on parameters with vector type):

define <4 x i1> @b(<4 x i32> %x, <4 x i32> %y) {
  %c1 = icmp eq <4 x i32> %x, <i32 42, i32 42, i32 42, i32 42>
  %c2 = icmp sgt <4 x i32> %y, <i32 42, i32 42, i32 42, i32 42>
  %s = select <4 x i1> %c1, <4 x i1> <i1 true, i1 true, i1 true, i1 true>, <4 x i1> %c2
  ret <4 x i1> %s
}
define <4 x i1> @b2(<4 x i32> %x, <4 x i32> %y) {
  %c1 = icmp eq <4 x i32> %x, <i32 42, i32 42, i32 42, i32 42>
  %c2 = icmp sgt <4 x i32> %y, <i32 42, i32 42, i32 42, i32 42>
  %s = or <4 x i1> %c1, %c2
  ret <4 x i1> %s
}

$ llc -o - logical.ll -mattr=avx -mtriple=x86_64--
_b:                                     ## @b
	vmovdqa	LCPI2_0(%rip), %xmm2            ## xmm2 = [42,42,42,42]
	vpcmpeqd	%xmm2, %xmm0, %xmm0
	vpcmpgtd	%xmm2, %xmm1, %xmm1
	vblendvps	%xmm0, LCPI2_1(%rip), %xmm1, %xmm0
	retq
_b2:                                    ## @b2
	vmovdqa	LCPI3_0(%rip), %xmm2            ## xmm2 = [42,42,42,42]
	vpcmpeqd	%xmm2, %xmm0, %xmm0
	vpcmpgtd	%xmm2, %xmm1, %xmm1
	vpor	%xmm1, %xmm0, %xmm0
	retq

Thank you for the info..!
I'll check whether regression happens with different targets as well and make patches if any.

select form of and/or operations are eagerly folded by InstCombine currently, so they'll look good hopefully.

[aqjune]

Would adding 'select -> and/or i1' transformation into the end of CodeGenPrepare::runOnFunction work?
I see that there are quite many targets that generate different assemblies for select vs. and/or i1.

[spatel]

llvm/include/llvm/Analysis/TargetTransformInfoImpl.h

1001

Assert that Op[0/1]->getType()->getScalarSizeInBits() == 1 ?

[spatel]

In D97360#2594420, @aqjune wrote:

Would adding 'select -> and/or i1' transformation into the end of CodeGenPrepare::runOnFunction work?
I see that there are quite many targets that generate different assemblies for select vs. and/or i1.

No, I think we should adjust some code in DAGCombiner for this. I was already looking at it, so I can continue if you want to keep working on the IR side.

LGTM.

[aqjune]

In D97360#2594461, @spatel wrote:

In D97360#2594420, @aqjune wrote:

Would adding 'select -> and/or i1' transformation into the end of CodeGenPrepare::runOnFunction work?
I see that there are quite many targets that generate different assemblies for select vs. and/or i1.

No, I think we should adjust some code in DAGCombiner for this. I was already looking at it, so I can continue if you want to keep working on the IR side.

LGTM.

This is great, thank you!

llvm/include/llvm/Analysis/TargetTransformInfoImpl.h

 //===- TargetTransformInfoImpl.h --------------------------------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 /// \file
 /// This file provides helpers for the implementation of
 /// a TargetTransformInfo-conforming class.
 ///
 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_ANALYSIS_TARGETTRANSFORMINFOIMPL_H
 #define LLVM_ANALYSIS_TARGETTRANSFORMINFOIMPL_H
 
 #include "llvm/Analysis/ScalarEvolutionExpressions.h"
 #include "llvm/Analysis/TargetTransformInfo.h"
 #include "llvm/Analysis/VectorUtils.h"
 #include "llvm/IR/DataLayout.h"
 #include "llvm/IR/Function.h"
 #include "llvm/IR/GetElementPtrTypeIterator.h"
 #include "llvm/IR/IntrinsicInst.h"
 #include "llvm/IR/Operator.h"
+#include "llvm/IR/PatternMatch.h"
 #include "llvm/IR/Type.h"
 
+using namespace llvm::PatternMatch;
+
 namespace llvm {
 
 /// Base class for use as a mix-in that aids implementing
 /// a TargetTransformInfo-compatible class.
 class TargetTransformInfoImplBase {
 protected:
   typedef TargetTransformInfo TTI;
 
int getUserCost(const User *U, ArrayRef<const Value *> Operands,
     }
     case Instruction::Load: {
       auto *LI = cast<LoadInst>(U);
       return TargetTTI->getMemoryOpCost(Opcode, U->getType(), LI->getAlign(),
                                         LI->getPointerAddressSpace(),
                                         CostKind, I);
     }
     case Instruction::Select: {
+      const Value *Op0, *Op1;
+      if (match(U, m_LogicalAnd(m_Value(Op0), m_Value(Op1))) ||
+          match(U, m_LogicalOr(m_Value(Op0), m_Value(Op1)))) {
+        TTI::OperandValueProperties Op1VP = TTI::OP_None;
+        TTI::OperandValueProperties Op2VP = TTI::OP_None;
+        TTI::OperandValueKind Op1VK = TTI::getOperandInfo(Op0, Op1VP);
+        TTI::OperandValueKind Op2VK = TTI::getOperandInfo(Op1, Op2VP);
+        SmallVector<const Value *, 2> Operands{Op0, Op1};
+        return TargetTTI->getArithmeticInstrCost(
+            match(U, m_LogicalOr()) ? Instruction::Or : Instruction::And,
+            Ty, CostKind, Op1VK, Op2VK, Op1VP, Op2VP, Operands, I);
+      }
       Type *CondTy = U->getOperand(0)->getType();
       return TargetTTI->getCmpSelInstrCost(Opcode, U->getType(), CondTy,
                                            CmpInst::BAD_ICMP_PREDICATE,
                                            CostKind, I);
     }
     case Instruction::ICmp:
     case Instruction::FCmp: {
       Type *ValTy = U->getOperand(0)->getType();