[PATCH] D121366: Allow ImproveChain to get past relaxed atomics

[Yoni Lavi via Phabricator via llvm-commits]

yoni-lavi added a comment.

I can definitely come up with a test case where the SDAG is different.

However, strictly speaking, weakening the chain dependencies doesn't ever "force" a change in the output instruction schedule.

So a good test would have to assert on the SDAG structure rather than the output schedule / machine instructions.

Is there an existing framework to assert on the SDAG for a given basic block in an example program?



================
Comment at: llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp:24066
+    if (const auto *AN = dyn_cast<AtomicSDNode>(N)) {
+      // PRE_INC / PRE_DEC allegedly not a relevant issue, can set this to zero
+      int64_t Offset = 0;
----------------
craig.topper wrote:
> I'm not sure what this comment is trying to say
It's related to the case above (LSBaseSDNode) where `Offset` is computed to account for PRE_INC / PRE_DEC.

If I understood it right, this way of representing ptr increments as part of the memory access itself does not exist for the "atomic" SDNodes. Hence the `Offset = 0` and this comment.




================
Comment at: llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp:24071
+      return {AN->isVolatile(), AN->isAtomic(),          AN->getBasePtr(),
+              Offset,           Optional<int64_t>(Size), AN->getMemOperand()};
+    }
----------------
RKSimon wrote:
> is this how clang-format formats the code?
yes. (the phabricator build if clang-format wants to re-format your code...)



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121366