[PATCH] Make addrspacecast (gep) do addrspacecast (gep) instead.

[Quentin Colombet]

  Hi Matt,

  Overall your patch LGTM with few minor changes (see inline comments).

  Thanks,
  -Quentin

  PS: When you create a phabricator, please add more context to your patch (e.g., svn diff --diff-cmd=diff -x -U999999). This would make the review easier.


================
Comment at: lib/Transforms/InstCombine/InstructionCombining.cpp:1270
@@ +1269,3 @@
+          return new BitCastInst(NewGEP, GEP.getType());
+        else
+          return new AddrSpaceCastInst(NewGEP, GEP.getType());
----------------
No need for a ‘else’.

================
Comment at: lib/Transforms/InstCombine/InstructionCombining.cpp:1305
@@ +1304,3 @@
+              return new BitCastInst(NewGEP, GEP.getType());
+            else
+              return new AddrSpaceCastInst(NewGEP, GEP.getType());
----------------
Ditto.

================
Comment at: lib/Transforms/InstCombine/InstructionCombining.cpp:1348
@@ +1347,3 @@
+              return new BitCastInst(NewGEP, GEP.getType());
+            else
+              return new AddrSpaceCastInst(NewGEP, GEP.getType());
----------------
Ditto.

================
Comment at: test/Transforms/InstCombine/cast.ll:950
@@ +949,3 @@
+; CHECK-NEXT: getelementptr [100 x double] addrspace(1)*
+; CHECK: addrspacecast double addrspace(1)*
+; CHECK-NEXT: load double addrspace(3)*
----------------
What is the reason for not using CHECK-NEXT here?

================
Comment at: test/Transforms/InstCombine/getelementptr.ll:745
@@ +744,3 @@
+; CHECK: getelementptr [100 x double]* %arr, i64 0, i64 %V
+; CHECK-NEXT: %t = addrspacecast double*
+  %cast = addrspacecast [100 x double]* %arr to i64 addrspace(3)*
----------------
I think that wouldn’t hurt to check that the destination space is 3.
Alternatively, you can check that the load still uses 3 as addrspace, like you did for the other tests.


http://llvm-reviews.chandlerc.com/D2596