[PATCH] D115113: [InstCombine] Do not combine atomic and non-atomic loads.

[Ricky Zhou via Phabricator via llvm-commits]

rickyz added inline comments.


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombinePHI.cpp:699
+    // Make sure all arguments are the same type of operation.
+    if (LI->isAtomic() != IsAtomic || LI->isVolatile() != IsVolatile ||
+        LI->getPointerAddressSpace() != LoadAddrSpace)
----------------
nikic wrote:
> I think you'd be better of just bailing on `LI->isAtomic()` here, just as the check on the initial load does. IsAtomic is always false here, and if that weren't the case, then this check would be insufficient, because it conflates all types of atomic loads, which might have different ordering or sync scope.
Agreed, thanks!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D115113/new/

https://reviews.llvm.org/D115113