[PATCH] D21421: [NVPTX] Improve lowering of byval args of device functions.

[Justin Lebar via llvm-commits]

jlebar added inline comments.

================
Comment at: lib/Target/NVPTX/NVPTXISelDAGToDAG.cpp:668
@@ +667,3 @@
+        ReplaceNode(N, Src.getOperand(0).getNode());
+        return;
+      }
----------------
I'm not sure this is the right place to do this transformation, as opposed to (say) at the machine instruction level.

My first approximation mental model for selection dag is that it does a reasonably direct translation of IR to machine instructions.  It's responsible for recognizing mapping constructions in IR to fast machine instructions (e.g. x86's various addressing modes), but optimizations that recognize that e.g. f(f^-1(x)) == x are less common in isel.  (Maybe someone will tell me this is wrong.)

So I'm not sure whether this belongs here.  Especially if it's a correctness transformation -- what if the source isn't directly a MoveParam?  Like, we moved the param to one reg, then moved it to another reg, then converted *that*.  Are we clear of the bug we're working around?  If this is just an optimization, at least it should be guarded so we don't run it at -O0.

I would also like to be explicit in our source (somewhere) exactly what is the nvptx bug that we're working around, so that this change doesn't just sit in our backend and confuse all hackers who come after us.  :)


http://reviews.llvm.org/D21421