[PATCH] D57702: [SelectionDAGBuilder] Add restrictions to EmitFuncArgumentDbgValue

[Bjorn Pettersson via Phabricator via llvm-commits]

bjope marked 2 inline comments as done.
bjope added inline comments.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp:5011
+    // We also want to check that the parameter value V actually maps to the
+    // argument described by Variable. If we for example have IR like this
+    //
----------------
aprantl wrote:
> I don't understand why this is necessary?
One missing piece in the example might be that the function on C level looks like this
```
void foo(long a, long b) {
  ...
  b = 123;
  ...
  b = a;
}
```
so `"b"` is an argument on C level, and `%a` and` %b` are function argument values.

The assignment `b = a` results in the last dbg.value where we say that `"b"` has the value of `%a`. So both the DIVariable and the value is associated with function arguments, but it is not correct to hoist this dbg.value to the function entry (for example hoisting above the dbg.value mapping `"b"` to `123`), because the assignment is not done at function entry.

So the above maps to the `@foo_other_param` test.


I also made an assumption that we could not get something like this:
```
define @foo(i32 %a, i32 %b)  {
      entry:
        call void @llvm.dbg.value(metadata i32 %b, "b",
        ...
        call void @llvm.dbg.value(metadata i32 123, "b"
        ...
        call void @llvm.dbg.value(metadata i32 %b, "b"
```
where "b" first gets the value %b (at the function entry), then is reassigned another value, and then gets the value %b again. Because then the value of "b" must saved somethere so the value in the last assignment of "b" will not be %b. Now I realize that the above pattern isn't that hard to trigger, just use an input like this:
```
extern void fie(long);

void foo(long a, long b) {
  long c = b;
  b = 123;
  fie(b);
  b = c;
  fie(b);
}
```

So we can't really consider a dbg.value as describing a function argument just because "the parameter value V actually maps to the argument described by Variable". I will remove that code comment.

The only solution I can imagine right now is the one that I have implemented (assuming that only the first dbg.value that describes an argument (on IR level) is a "function argument dbg.value").


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D57702