From be687aff052bbdd6fd2cc3c75c8cebdd9ce7fab2 Mon Sep 17 00:00:00 2001 From: Nikita Popov Date: Fri, 10 Mar 2023 10:34:27 +0100 Subject: [PATCH] [InstSimplify] Adjust context instruction when threading phi (PR61312) When threading operations over phis, we need to adjust the context instruction to the terminator of the incoming block. This was handled when threading icmps, but not when threading binops. Fixes https://github.com/llvm/llvm-project/issues/61312. --- llvm/lib/Analysis/InstructionSimplify.cpp | 10 +++++++--- llvm/test/Transforms/InstSimplify/phi.ll | 6 ++++-- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/llvm/lib/Analysis/InstructionSimplify.cpp b/llvm/lib/Analysis/InstructionSimplify.cpp index abcde1d..c57a111 100644 --- a/llvm/lib/Analysis/InstructionSimplify.cpp +++ b/llvm/lib/Analysis/InstructionSimplify.cpp @@ -539,12 +539,16 @@ static Value *threadBinOpOverPHI(Instruction::BinaryOps Opcode, Value *LHS, // Evaluate the BinOp on the incoming phi values. Value *CommonValue = nullptr; - for (Value *Incoming : PI->incoming_values()) { + for (Use &Incoming : PI->incoming_values()) { // If the incoming value is the phi node itself, it can safely be skipped. if (Incoming == PI) continue; - Value *V = PI == LHS ? simplifyBinOp(Opcode, Incoming, RHS, Q, MaxRecurse) - : simplifyBinOp(Opcode, LHS, Incoming, Q, MaxRecurse); + Instruction *InTI = PI->getIncomingBlock(Incoming)->getTerminator(); + Value *V = PI == LHS + ? simplifyBinOp(Opcode, Incoming, RHS, + Q.getWithInstruction(InTI), MaxRecurse) + : simplifyBinOp(Opcode, LHS, Incoming, + Q.getWithInstruction(InTI), MaxRecurse); // If the operation failed to simplify, or simplified to a different value // to previously, then give up. if (!V || (CommonValue && V != CommonValue)) diff --git a/llvm/test/Transforms/InstSimplify/phi.ll b/llvm/test/Transforms/InstSimplify/phi.ll index 14f1767..c97326e 100644 --- a/llvm/test/Transforms/InstSimplify/phi.ll +++ b/llvm/test/Transforms/InstSimplify/phi.ll @@ -153,7 +153,8 @@ EXIT: ret i8 %r } -; FIXME: This is a miscompile. +; Should not fold srem to -1 due to incorrect context instruction when +; threading over phi. define i32 @pr61312() { ; CHECK-LABEL: @pr61312( ; CHECK-NEXT: entry: @@ -166,7 +167,8 @@ define i32 @pr61312() { ; CHECK-NEXT: [[DEC]] = add nsw i32 [[A_0]], -1 ; CHECK-NEXT: br label [[FOR_COND]] ; CHECK: for.end: -; CHECK-NEXT: ret i32 -1 +; CHECK-NEXT: [[REM:%.*]] = srem i32 -1, [[A_0]] +; CHECK-NEXT: ret i32 [[REM]] ; entry: br label %for.cond -- 2.7.4