diff --git a/llvm/lib/Transforms/InstCombine/InstCombinePHI.cpp b/llvm/lib/Transforms/InstCombine/InstCombinePHI.cpp --- a/llvm/lib/Transforms/InstCombine/InstCombinePHI.cpp +++ b/llvm/lib/Transforms/InstCombine/InstCombinePHI.cpp @@ -667,9 +667,9 @@ // don't sink loads when some have their alignment specified and some don't. // visitLoadInst will propagate an alignment onto the load when TD is around, // and if TD isn't around, we can't handle the mixed case. - bool IsVolatile = FirstLI->isVolatile(); + const bool IsVolatile = FirstLI->isVolatile(); Align LoadAlignment = FirstLI->getAlign(); - unsigned LoadAddrSpace = FirstLI->getPointerAddressSpace(); + const unsigned LoadAddrSpace = FirstLI->getPointerAddressSpace(); // We can't sink the load if the loaded value could be modified between the // load and the PHI. @@ -684,19 +684,21 @@ FirstLI->getParent()->getTerminator()->getNumSuccessors() != 1) return nullptr; - // Check to see if all arguments are the same operation. for (auto Incoming : drop_begin(zip(PN.blocks(), PN.incoming_values()))) { BasicBlock *InBB = std::get<0>(Incoming); Value *InVal = std::get<1>(Incoming); LoadInst *LI = dyn_cast(InVal); - if (!LI || !LI->hasOneUser()) + if (!LI || !LI->hasOneUser() || LI->isAtomic()) + return nullptr; + + // Make sure all arguments are the same type of operation. + if (LI->isVolatile() != IsVolatile || + LI->getPointerAddressSpace() != LoadAddrSpace) return nullptr; // We can't sink the load if the loaded value could be modified between // the load and the PHI. - if (LI->isVolatile() != IsVolatile || LI->getParent() != InBB || - LI->getPointerAddressSpace() != LoadAddrSpace || - !isSafeAndProfitableToSinkLoad(LI)) + if (LI->getParent() != InBB || !isSafeAndProfitableToSinkLoad(LI)) return nullptr; LoadAlignment = std::min(LoadAlignment, Align(LI->getAlign())); diff --git a/llvm/test/Transforms/InstCombine/phi.ll b/llvm/test/Transforms/InstCombine/phi.ll --- a/llvm/test/Transforms/InstCombine/phi.ll +++ b/llvm/test/Transforms/InstCombine/phi.ll @@ -547,6 +547,29 @@ ret i32 %res } +; Atomic and non-atomic loads should not be combined. +define i32 @PR51435(i32* %ptr, i32* %atomic_ptr, i1 %c) { +; CHECK-LABEL: @PR51435( +; CHECK: entry: +; CHECK-NEXT: [[NON_ATOMIC:%.*]] = load i32, i32* %ptr, align 4 +; CHECK: if: +; CHECK-NEXT: [[ATOMIC:%.*]] = load atomic i32, i32* %atomic_ptr acquire, align 4 +; CHECK: end: +; CHECK-NEXT: [[COND:%.*]] = phi i32 [ [[NON_ATOMIC]], %entry ], [ [[ATOMIC]], %if ] +; CHECK-NEXT: ret i32 [[COND]] +entry: + %x = load i32, i32* %ptr, align 4 + br i1 %c, label %if, label %end + +if: + %y = load atomic i32, i32* %atomic_ptr acquire, align 4 + br label %end + +end: + %cond = phi i32 [ %x, %entry ], [ %y, %if ] + ret i32 %cond +} + define i1 @test18(i1 %cond) { ; CHECK-LABEL: @test18( ; CHECK-NEXT: br i1 [[COND:%.*]], label [[TRUE:%.*]], label [[FALSE:%.*]]