[PATCH] D102096: [DAGCombiner] Fix DAG combine store elimination, different address space.

Min-Yih Hsu via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Sun May 9 21:30:40 PDT 2021


myhsu added a comment.

This patch looks legit to me.
I'll suggest you to put people that have been actively working on this part as reviewers. They might have some other comments.



================
Comment at: llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp:17920
+        Ld->getPointerInfo().getAddrSpace() ==
+            ST->getPointerInfo().getAddrSpace() &&
         // There can't be any side effects between the load and store, such as
----------------
just curious, is the code here formatted by clang-format?


================
Comment at: llvm/test/CodeGen/X86/fs_gs_dag_combine.ll:18
+; CHECK-NEXT: movl %eax, %fs:2
+define i32 @copy_fs_diff() {
+entry:
----------------
copy_fs_diff and copy_gs_diff seem a little redundant to me: The address spaces are already different, why should we care about address? Can you elaborate a little more on the purpose of these two functions?


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

https://reviews.llvm.org/D102096



More information about the llvm-commits mailing list