From 26f6ec3e16ac4e2884e5ec56af13c8ab68998163 Mon Sep 17 00:00:00 2001 From: Philip Reames Date: Thu, 2 Nov 2023 11:19:19 -0700 Subject: [PATCH 1/2] [instcombine] Drop zext nneg flag when simplify operand This fixes a miscompile introduced in the recent #67982, and likely exposed in changes since to infer and leverage the same. No active bug reports as of yet. This was noticed in https://github.com/llvm/llvm-project/pull/70858#discussion_r1378203532. --- .../Transforms/InstCombine/InstCombineSimplifyDemanded.cpp | 6 +++++- llvm/test/Transforms/InstCombine/zext.ll | 2 +- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/llvm/lib/Transforms/InstCombine/InstCombineSimplifyDemanded.cpp b/llvm/lib/Transforms/InstCombine/InstCombineSimplifyDemanded.cpp index fa6fe9d30abd1..30c1565ab44f7 100644 --- a/llvm/lib/Transforms/InstCombine/InstCombineSimplifyDemanded.cpp +++ b/llvm/lib/Transforms/InstCombine/InstCombineSimplifyDemanded.cpp @@ -422,8 +422,12 @@ Value *InstCombinerImpl::SimplifyDemandedUseBits(Value *V, APInt DemandedMask, APInt InputDemandedMask = DemandedMask.zextOrTrunc(SrcBitWidth); KnownBits InputKnown(SrcBitWidth); - if (SimplifyDemandedBits(I, 0, InputDemandedMask, InputKnown, Depth + 1)) + if (SimplifyDemandedBits(I, 0, InputDemandedMask, InputKnown, Depth + 1)) { + // For zext nneg, we may have dropped the instruction which made the + // input non-negative. + I->dropPoisonGeneratingFlags(); return I; + } assert(InputKnown.getBitWidth() == SrcBitWidth && "Src width changed?"); Known = InputKnown.zextOrTrunc(BitWidth); assert(!Known.hasConflict() && "Bits known to be one AND zero?"); diff --git a/llvm/test/Transforms/InstCombine/zext.ll b/llvm/test/Transforms/InstCombine/zext.ll index 9dd0d929530b3..4034bc3c531b4 100644 --- a/llvm/test/Transforms/InstCombine/zext.ll +++ b/llvm/test/Transforms/InstCombine/zext.ll @@ -783,7 +783,7 @@ define i64 @evaluate_zexted_const_expr(i1 %c) { ; but the flag on the zext doesn't. define i16 @zext_nneg_flag_drop(i8 %x, i16 %y) { ; CHECK-LABEL: @zext_nneg_flag_drop( -; CHECK-NEXT: [[EXT:%.*]] = zext nneg i8 [[X:%.*]] to i16 +; CHECK-NEXT: [[EXT:%.*]] = zext i8 [[X:%.*]] to i16 ; CHECK-NEXT: [[OR1:%.*]] = or i16 [[EXT]], [[Y:%.*]] ; CHECK-NEXT: [[OR2:%.*]] = or i16 [[OR1]], 128 ; CHECK-NEXT: ret i16 [[OR2]] From dc64d3aeb6ba4f6a3fadab6da49bc5213415ae43 Mon Sep 17 00:00:00 2001 From: Philip Reames Date: Thu, 2 Nov 2023 11:43:23 -0700 Subject: [PATCH 2/2] Update zext.ll (drop fixme) --- llvm/test/Transforms/InstCombine/zext.ll | 2 -- 1 file changed, 2 deletions(-) diff --git a/llvm/test/Transforms/InstCombine/zext.ll b/llvm/test/Transforms/InstCombine/zext.ll index 4034bc3c531b4..5a00b7575d70f 100644 --- a/llvm/test/Transforms/InstCombine/zext.ll +++ b/llvm/test/Transforms/InstCombine/zext.ll @@ -779,8 +779,6 @@ define i64 @evaluate_zexted_const_expr(i1 %c) { ret i64 %ext } -; FIXME: This is currently miscompiling as the and gets dropped, -; but the flag on the zext doesn't. define i16 @zext_nneg_flag_drop(i8 %x, i16 %y) { ; CHECK-LABEL: @zext_nneg_flag_drop( ; CHECK-NEXT: [[EXT:%.*]] = zext i8 [[X:%.*]] to i16