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 @@ -662,7 +662,8 @@ // FIXME: This is overconservative; this transform is allowed in some cases // for atomic operations. - if (FirstLI->isAtomic()) + const bool IsAtomic = FirstLI->isAtomic(); + if (IsAtomic) return nullptr; // When processing loads, we need to propagate two bits of information to the @@ -670,9 +671,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. @@ -687,7 +688,6 @@ 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); @@ -695,11 +695,14 @@ if (!LI || !LI->hasOneUser()) return nullptr; + // Make sure all arguments are the same type of operation. + if (LI->isAtomic() != IsAtomic || 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:%.*]]