[PATCH] D43154: [X86] Correct load-op-store cycle detection Analysis.

[James Y Knight via Phabricator via llvm-commits]

jyknight added a comment.

Comments are very confusing, but logic seems reasonable.



================
Comment at: llvm/lib/Target/X86/X86ISelDAGToDAG.cpp:2109
+
+  //  Visualization of Merge:
+  // -------------------------
----------------
"Visualization of load-store-fuse transformation"


================
Comment at: llvm/lib/Target/X86/X86ISelDAGToDAG.cpp:2137
+  //  * a. if X, Yn are sucessors to LD
+  //  * b. if Zn is a sucessor to ST.
+
----------------
backwards, if Zn is a chain predecessor to St, but a value successor to Op.


================
Comment at: llvm/lib/Target/X86/X86ISelDAGToDAG.cpp:2139
+
+  // We can further simplify b by noting that for ST to be a
+  // predecessor to Zn X, TF, LD, OP, or Yn must be a predecessor (or
----------------
Too confusing, rewrite.


================
Comment at: llvm/test/CodeGen/X86/pr36274.ll:41
+  %0 = load i32, i32* @g, align 4
+  %1 = load i64, i64* getelementptr inbounds ([13 x [2 x i64]], [13 x [2 x i64]]* @a, i32 0, i32 0, i32 0), align 8
+  %inc = add i64 %1, 1
----------------
I think a single load i64, add i64, then volatile store of the low half, volatile store of the high half might show the same issue with less cruft in the test.


Repository:
  rL LLVM

https://reviews.llvm.org/D43154