[PATCH] D58831: [DebugInfo] Ignore bitcasts when lowering stack arg dbg.values

[Bjorn Pettersson via Phabricator via llvm-commits]

bjope added inline comments.


================
Comment at: lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp:5309
     // Check if frame index is available.
-    if (LoadSDNode *LNode = dyn_cast<LoadSDNode>(N.getNode()))
+    SDValue LCandidate = peekThroughBitcasts(N);
+    if (LoadSDNode *LNode = dyn_cast<LoadSDNode>(LCandidate.getNode()))
----------------
aprantl wrote:
> LGTM; is this the most general place where to do this, or could we do this higher up in the function and catch more cases?
I actually asked for a test case when the argument was in a register (hitting the uses of N above) when discussing this with David offline. Just to see if we were missing out on doing the same thing higher up in the function.
The answer from David was that there already is a test case for that scenario. We already peek-through-bitcast in getUnderlyingArgReg above, and there is a test case triggering that scenario.

One could argue that we should use getUnderlyingArgReg here as well. However, that one is also dealing with AssertSext etc. We have no test cases verifying correctness for those non-BITCAST-peek-through's done by getUnderlyingArgReg  (maybe it is impossible to get trigger such situations here). So it feels more safe to only focus on BITCAST here right now.

So, LGTM!


Repository:
  rL LLVM

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

https://reviews.llvm.org/D58831