[InstCombine] Support sext in foldLogicCastConstant

[craig.topper]

Summary

This adds support for sext in foldLogicCastConstant. This is a prerequisite for D36214.

Not clear if the test changes are a win or not.

Could reduce the handling to only do all 0s/1s constants which would be sufficient for what's needed in D36214.

[spatel]

LGTM. I think it makes sense for the same reason as the zext case.

lib/Transforms/InstCombine/InstCombineAndOrXor.cpp

1092 ↗

(On Diff #109381)

Update comment to 'zext or sext'.

test/Transforms/InstCombine/cast.ll

591–592 ↗

(On Diff #109381)

Not this patch, but seems odd that we'd choose to sext to the larger type and mask rather than sext+zext?

[eastig]

Hi,

This patch causes an assertion failure on the attached test.ll. Could you please have a look at it?

$ ../bin/opt test.ll -instcombine -S
opt: ../lib/IR/Constants.cpp:1551: static llvm::Constant* llvm::ConstantExpr::getTrunc(llvm::Constant*, llvm::Type*, bool): Assertion `C->getType()->getScalarSizeInBits() > Ty->getScalarSizeInBits()&& "SrcTy must be larger than DestTy for Trunc!"' failed.
#0 0x000000000360ab38 llvm::sys::PrintStackTrace(llvm::raw_ostream&) /home/evgast01/local/repos/llvm12/build.d/../lib/Support/Unix/Signals.inc:398:0
#1 0x000000000360abc9 PrintStackTraceSignalHandler(void*) /home/evgast01/local/repos/llvm12/build.d/../lib/Support/Unix/Signals.inc:462:0
#2 0x0000000003608fde llvm::sys::RunSignalHandlers() /home/evgast01/local/repos/llvm12/build.d/../lib/Support/Signals.cpp:49:0
#3 0x000000000360a4bb SignalHandler(int) /home/evgast01/local/repos/llvm12/build.d/../lib/Support/Unix/Signals.inc:252:0
#4 0x00007f37c51e1330 __restore_rt (/lib/x86_64-linux-gnu/libpthread.so.0+0x10330)
#5 0x00007f37c3fc9c37 gsignal (/lib/x86_64-linux-gnu/libc.so.6+0x36c37)
#6 0x00007f37c3fcd028 abort (/lib/x86_64-linux-gnu/libc.so.6+0x3a028)
#7 0x00007f37c3fc2bf6 (/lib/x86_64-linux-gnu/libc.so.6+0x2fbf6)
#8 0x00007f37c3fc2ca2 (/lib/x86_64-linux-gnu/libc.so.6+0x2fca2)
#9 0x0000000002e19bc5 llvm::ConstantExpr::getTrunc(llvm::Constant*, llvm::Type*, bool) /home/evgast01/local/repos/llvm12/build.d/../lib/IR/Constants.cpp:1553:0
#10 0x0000000003103c73 getSelectCondition(llvm::Value*, llvm::Value*, llvm::IRBuilder<llvm::TargetFolder, llvm::IRBuilderCallbackInserter>&) /home/evgast01/local/repos/llvm12/build.d/../lib/Transforms/InstCombine/InstCombineAndOrXor.cpp:1498:0
#11 0x0000000003103f10 matchSelectFromAndOr(llvm::Value*, llvm::Value*, llvm::Value*, llvm::Value*, llvm::IRBuilder<llvm::TargetFolder, llvm::IRBuilderCallbackInserter>&) /home/evgast01/local/repos/llvm12/build.d/../lib/Transforms/InstCombine/InstCombineAndOrXor.cpp:1522:0
#12 0x00000000031071fe llvm::InstCombiner::visitOr(llvm::BinaryOperator&) /home/evgast01/local/repos/llvm12/build.d/../lib/Transforms/InstCombine/InstCombineAndOrXor.cpp:1989:0
#13 0x00000000030e9dbb llvm::InstVisitor<llvm::InstCombiner, llvm::Instruction*>::visit(llvm::Instruction&) /home/evgast01/local/repos/llvm12/build.d/../include/llvm/IR/Instruction.def:146:0
#14 0x00000000030e48a5 llvm::InstCombiner::run() /home/evgast01/local/repos/llvm12/build.d/../lib/Transforms/InstCombine/InstructionCombining.cpp:2971:0
#15 0x00000000030e5a86 combineInstructionsOverFunction(llvm::Function&, llvm::InstCombineWorklist&, llvm::AAResults*, llvm::AssumptionCache&, llvm::TargetLibraryInfo&, llvm::DominatorTree&, llvm::OptimizationRemarkEmitter&, bool, llvm::LoopInfo*) /home/evgast01/local/repos/llvm12/build.d/../lib/Transforms/InstCombine/InstructionCombining.cpp:3203:0
#16 0x00000000030e5e0f llvm::InstructionCombiningPass::runOnFunction(llvm::Function&) /home/evgast01/local/repos/llvm12/build.d/../lib/Transforms/InstCombine/InstructionCombining.cpp:3262:0
#17 0x0000000002f14f46 llvm::FPPassManager::runOnFunction(llvm::Function&) /home/evgast01/local/repos/llvm12/build.d/../lib/IR/LegacyPassManager.cpp:1514:0
#18 0x0000000002f150d9 llvm::FPPassManager::runOnModule(llvm::Module&) /home/evgast01/local/repos/llvm12/build.d/../lib/IR/LegacyPassManager.cpp:1535:0
#19 0x0000000002f15474 (anonymous namespace)::MPPassManager::runOnModule(llvm::Module&) /home/evgast01/local/repos/llvm12/build.d/../lib/IR/LegacyPassManager.cpp:1591:0
#20 0x0000000002f15bcb llvm::legacy::PassManagerImpl::run(llvm::Module&) /home/evgast01/local/repos/llvm12/build.d/../lib/IR/LegacyPassManager.cpp:1694:0
#21 0x0000000002f15e0b llvm::legacy::PassManager::run(llvm::Module&) /home/evgast01/local/repos/llvm12/build.d/../lib/IR/LegacyPassManager.cpp:1726:0
#22 0x00000000015ee1ae main /home/evgast01/local/repos/llvm12/build.d/../tools/opt/opt.cpp:759:0
#23 0x00007f37c3fb4f45 __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x21f45)
#24 0x00000000015c7f89 _start (../bin/opt+0x15c7f89)
Stack dump:
0.      Program arguments: ../bin/opt test.ll -instcombine -S 
1.      Running pass 'Function Pass Manager' on module 'test.ll'.
2.      Running pass 'Combine redundant instructions' on function '@dotests_115'
Aborted

test.ll688 BDownload

[craig.topper]

Fixed the assertion failure in r310639. Looks like a latent bug that was always there. I added a reduced test case with the commit that doesn't rely on any sext folding.

[eastig]

In D36234#838302, @craig.topper wrote:

Fixed the assertion failure in r310639. Looks like a latent bug that was always there. I added a reduced test case with the commit that doesn't rely on any sext folding.

Thank you, Craig, for fixing it quickly.

llvm/trunk/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp

static Instruction *foldLogicCastConstant(BinaryOperator &Logic, CastInst *Cast,
   Constant *C;
   if (!match(Logic.getOperand(1), m_Constant(C)))
     return nullptr;
 
   auto LogicOpc = Logic.getOpcode();
   Type *DestTy = Logic.getType();
   Type *SrcTy = Cast->getSrcTy();
 
-  // Move the logic operation ahead of a zext if the constant is unchanged in
-  // the smaller source type. Performing the logic in a smaller type may provide
-  // more information to later folds, and the smaller logic instruction may be
-  // cheaper (particularly in the case of vectors).
+  // Move the logic operation ahead of a zext or sext if the constant is
+  // unchanged in the smaller source type. Performing the logic in a smaller
+  // type may provide more information to later folds, and the smaller logic
+  // instruction may be cheaper (particularly in the case of vectors).
   Value *X;
   if (match(Cast, m_OneUse(m_ZExt(m_Value(X))))) {
     Constant *TruncC = ConstantExpr::getTrunc(C, SrcTy);
     Constant *ZextTruncC = ConstantExpr::getZExt(TruncC, DestTy);
     if (ZextTruncC == C) {
       // LogicOpc (zext X), C --> zext (LogicOpc X, C)
       Value *NewOp = Builder.CreateBinOp(LogicOpc, X, TruncC);
       return new ZExtInst(NewOp, DestTy);
     }
   }
 
+  if (match(Cast, m_OneUse(m_SExt(m_Value(X))))) {
+    Constant *TruncC = ConstantExpr::getTrunc(C, SrcTy);
+    Constant *SextTruncC = ConstantExpr::getSExt(TruncC, DestTy);
+    if (SextTruncC == C) {
+      // LogicOpc (sext X), C --> sext (LogicOpc X, C)
+      Value *NewOp = Builder.CreateBinOp(LogicOpc, X, TruncC);
+      return new SExtInst(NewOp, DestTy);
+    }
+  }
+
   return nullptr;
 }
 
 /// Fold {and,or,xor} (cast X), Y.
 Instruction *InstCombiner::foldCastedBitwiseLogic(BinaryOperator &I) {
   auto LogicOpc = I.getOpcode();
   assert(I.isBitwiseLogicOp() && "Unexpected opcode for bitwise logic folding");
 

llvm/trunk/test/Transforms/InstCombine/cast.ll

;
   %C = and i32 %B, 42
   %D = shl i32 %C, 8
   %E = zext i32 %D to i64
   ret i64 %E
 }
 
 define i64 @test47(i8 %A) {
 ; CHECK-LABEL: @test47(
-; CHECK-NEXT:    [[B:%.*]] = sext i8 %A to i64
-; CHECK-NEXT:    [[C:%.*]] = and i64 [[B]], 4294967253
-; CHECK-NEXT:    [[E:%.*]] = or i64 [[C]], 42
+; CHECK-NEXT:    [[TMP1:%.*]] = or i8 [[A:%.*]], 42
+; CHECK-NEXT:    [[C:%.*]] = sext i8 [[TMP1]] to i64
+; CHECK-NEXT:    [[E:%.*]] = and i64 [[C]], 4294967295
 ; CHECK-NEXT:    ret i64 [[E]]
 ;
   %B = sext i8 %A to i32
   %C = or i32 %B, 42
   %E = zext i32 %C to i64
   ret i64 %E
 }
 

llvm/trunk/test/Transforms/InstCombine/logical-select.ll

;
   ret <4 x i32> %or
 }
 
 ; Allow the transform even if the mask values have multiple uses because
 ; there's still a net reduction of instructions from removing the and/and/or.
 
 define <4 x i32> @vec_sel_xor_multi_use(<4 x i32> %a, <4 x i32> %b, <4 x i1> %c) {
 ; CHECK-LABEL: @vec_sel_xor_multi_use(
-; CHECK-NEXT:    [[MASK:%.*]] = sext <4 x i1> %c to <4 x i32>
-; CHECK-NEXT:    [[MASK_FLIP1:%.*]] = xor <4 x i32> [[MASK]], <i32 -1, i32 0, i32 0, i32 0>
-; CHECK-NEXT:    [[TMP1:%.*]] = xor <4 x i1> %c, <i1 false, i1 true, i1 true, i1 true>
-; CHECK-NEXT:    [[TMP2:%.*]] = select <4 x i1> [[TMP1]], <4 x i32> %a, <4 x i32> %b
-; CHECK-NEXT:    [[ADD:%.*]] = add <4 x i32> [[TMP2]], [[MASK_FLIP1]]
+; CHECK-NEXT:    [[TMP1:%.*]] = xor <4 x i1> [[C:%.*]], <i1 true, i1 false, i1 false, i1 false>
+; CHECK-NEXT:    [[MASK_FLIP1:%.*]] = sext <4 x i1> [[TMP1]] to <4 x i32>
+; CHECK-NEXT:    [[TMP2:%.*]] = xor <4 x i1> [[C]], <i1 false, i1 true, i1 true, i1 true>
+; CHECK-NEXT:    [[TMP3:%.*]] = select <4 x i1> [[TMP2]], <4 x i32> [[A:%.*]], <4 x i32> [[B:%.*]]
+; CHECK-NEXT:    [[ADD:%.*]] = add <4 x i32> [[TMP3]], [[MASK_FLIP1]]
 ; CHECK-NEXT:    ret <4 x i32> [[ADD]]
 ;
   %mask = sext <4 x i1> %c to <4 x i32>
   %mask_flip1 = xor <4 x i32> %mask, <i32 -1, i32 0, i32 0, i32 0>
   %not_mask_flip1 = xor <4 x i32> %mask, <i32 0, i32 -1, i32 -1, i32 -1>
   %and1 = and <4 x i32> %not_mask_flip1, %a
   %and2 = and <4 x i32> %mask_flip1, %b
   %or = or <4 x i32> %and1, %and2
   %add = add <4 x i32> %or, %mask_flip1
   ret <4 x i32> %add
 }
 

llvm/trunk/test/Transforms/SLPVectorizer/X86/minimum-sizes.ll

 ;        In general, we need to add an extra bit to the maximum bit width only
 ;        if we can't prove that the upper bit of the original type is equal to
 ;        the upper bit of the proposed smaller type. If these two bits are the
 ;        same (either zero or one) we know that sign-extending from the smaller
 ;        type will result in the same value. Since we don't yet perform this
 ;        optimization, we make the proposed smaller type (i8) larger (i16) to
 ;        ensure correctness.
 ;
-; CHECK: %[[S0:.+]] = sext <2 x i8> {{.*}} to <2 x i16>
-; CHECK: %[[OR:.+]] = or <2 x i16> %[[S0]], <i16 1, i16 1>
-; CHECK: %[[E0:.+]] = extractelement <2 x i16> %[[OR]], i32 0
+; CHECK: %[[OR:.+]] = or <2 x i8> {{.*}}, <i8 1, i8 1>
+; CHECK: %[[S0:.+]] = sext <2 x i8> %[[OR]] to <2 x i16>
+; CHECK: %[[E0:.+]] = extractelement <2 x i16> %[[S0]], i32 0
 ; CHECK: %[[S1:.+]] = sext i16 %[[E0]] to i64
 ; CHECK: getelementptr inbounds i8, i8* %ptr, i64 %[[S1]]
-; CHECK: %[[E1:.+]] = extractelement <2 x i16> %[[OR]], i32 1
+; CHECK: %[[E1:.+]] = extractelement <2 x i16> %[[S0]], i32 1
 ; CHECK: %[[S2:.+]] = sext i16 %[[E1]] to i64
 ; CHECK: getelementptr inbounds i8, i8* %ptr, i64 %[[S2]]
 ;
 define i8 @PR31243_sext(i8 %v0, i8 %v1, i8 %v2, i8 %v3, i8* %ptr) {
 entry:
   %tmp0 = sext i8 %v0 to i32
   %tmp1 = sext i8 %v1 to i32
   %tmp2 = or i32 %tmp0, 1
   %tmp3 = or i32 %tmp1, 1
   %tmp4 = getelementptr inbounds i8, i8* %ptr, i32 %tmp2
   %tmp5 = getelementptr inbounds i8, i8* %ptr, i32 %tmp3
   %tmp6 = load i8, i8* %tmp4
   %tmp7 = load i8, i8* %tmp5
   %tmp8 = add i8 %tmp6, %tmp7
   ret i8 %tmp8
 }