[PATCH] D118607: [InstCombine] Remove weaker fence adjacent to a stronger fence

[Philip Reames via Phabricator via llvm-commits]

reames requested changes to this revision.
reames added inline comments.
This revision now requires changes to proceed.


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp:2473
   // Remove identical consecutive fences.
-  Instruction *Next = FI.getNextNonDebugInstruction();
-  if (auto *NFI = dyn_cast<FenceInst>(Next))
-    if (FI.isIdenticalTo(NFI))
+  if (NFI && FI.isIdenticalTo(NFI))
+    return eraseInstFromFunction(FI);
----------------
As I said before, I don't believe this check serves any purpose after the addition of the new transform.  


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp:2486
+
+  if (NFI && isSystemScopeFence(NFI) &&
+      isStrongerThan(NFI->getOrdering(), FI.getOrdering()))
----------------
Style suggestion: pull out a isStrongerFence(FI1,FI2) helper which does both scope and ordering check, then call it twice.  I believe the resulting code should be easier to read.  Can be a lambda or static function as you prefer.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118607