[PATCH] D11774: [NVPTX] Use LDG for pointer induction variables

[Bjarke Hammersholt Roune]

broune added inline comments.

================
Comment at: lib/Target/NVPTX/NVPTXISelDAGToDAG.cpp:551
@@ +550,3 @@
+                          unsigned codeAddrSpace, const DataLayout &DL,
+                          MachineFunction *F) {
+  if (!Subtarget.hasLDG() || codeAddrSpace != NVPTX::PTXLdStInstCode::GLOBAL ||
----------------
eliben wrote:
> Can't you get the data layout from F now?
Good point. Done.

================
Comment at: lib/Target/NVPTX/NVPTXISelDAGToDAG.cpp:561
@@ +560,3 @@
+  // nodes to handle pointer induction variables.
+  SmallVector<Value *, 8> objs;
+  GetUnderlyingObjects(const_cast<Value *>(N->getMemOperand()->getValue()),
----------------
eliben wrote:
> maybe Objs for coding style (elsewhere too)?
Done. Actually lots of variables in this file are leading-lower-case independent of my change. I'll fix that later, but it will change lots of lines, so I'll keep it separate from this change.

================
Comment at: lib/Target/NVPTX/NVPTXISelDAGToDAG.cpp:562
@@ +561,3 @@
+  SmallVector<Value *, 8> objs;
+  GetUnderlyingObjects(const_cast<Value *>(N->getMemOperand()->getValue()),
+                       objs, DL);
----------------
eliben wrote:
> How come const_cast is needed now but not before?
GetUnderlyingObject has an overload that takes a const Value * and just wraps the non-const version using const_cast. There is no such overload for GetUnderlyingObjects, which makes sense, since one of the parameters is a container of Value * that would have to become a container of const Value *. Converting between such containers is problematic. So I think it's reasonable not to have the const overload, even though it does push const_cast into the callers. I checked previously what other callers of GetUnderlyingObjects do in this case and they also use const_cast.


http://reviews.llvm.org/D11774