[PATCH] D17093: [X86] Add address space for SS segment

Michael Kuperstein via llvm-commits llvm-commits at lists.llvm.org
Thu Apr 21 18:09:55 PDT 2016


mkuper added inline comments.

================
Comment at: lib/Target/X86/X86ISelDAGToDAG.cpp:1427
@@ -1426,3 +1426,3 @@
   unsigned AddrSpace = Mgs->getPointerInfo().getAddrSpace();
-  // AddrSpace 256 -> GS, 257 -> FS.
+  // AddrSpace 256 -> GS, 257 -> FS, 258 -> SS.
   if (AddrSpace == 256)
----------------
There's also similar code in X86DAGToDAGISel::matchLoadInAddress().
Did you leave that unchanged on purpose?

If yes, perhaps it's worth adding a comment there for why. If not, perhaps factor it out into a separate function instead of having it duplicated 3 times?

================
Comment at: test/CodeGen/X86/movss.ll:5
@@ +4,3 @@
+entry:
+; CHECK: ss
+	%tmp = load i32*, i32* addrspace(258)* getelementptr (i32*, i32* addrspace(258)* inttoptr (i32 72 to i32* addrspace(258)*), i32 31)		; <i32*> [#uses=1]
----------------
Can you make the test slightly more explicit?
The current check would match a "movss" instruction, for example.

Also, it would be nice to make it slightly smaller, e.g. something like


```
define i32 @foo(i32 addrspace(258)* %p) {
  %tmp = load i32, i32 addrspace(258)* %p
  ret i32 %tmp
}
```


http://reviews.llvm.org/D17093





More information about the llvm-commits mailing list