[PATCH] D53758: Handle value uses wrapped in metadata for the use-list order

[David Stenberg via Phabricator via llvm-commits]

dstenb added inline comments.


================
Comment at: test/Assembler/metadata-use-uselistorder.ll:5-6
+
+; Verify that the uses of i64 0 in the llvm.dbg.value operands are considered
+; when generating the use-list order.
+
----------------
dexonsmith wrote:
> Does this only cause problems for constants (like `i64 0`), or also for use lists of locals and globals?
> 
> The reason I ask: we *should* make it impossible for most constants to rely on use-list order, and then stop storing use-lists for them (in either bitcode or textual IR).  When we get there, should we delete this test and the new code you've added?  Or will it still be relevant for other values?
Oh, okay! Do you know if someone is actively working on that at the moment?

The metadata operands can affect the use lists of globals in cases where they contain constant expressions using globals.

For example, the following file:

```
typedef struct { int a; int b; int c; } S;

extern void work(int *);

S g;

void foo() {
  int *local1 = &g.a;
}

void bar() {
  int *local2 = &g.b;
  work(local2);
  S l = {1, 2, 3};
  g = l;
}
```

compiled using the following, slightly contrived, command line:

```
$ clang -O0 -Xclang -disable-O0-optnone -g -S -emit-llvm di.c -o - | opt -S -instcombine -o di.ll
$ grep '@g' di.ll
@g = common dso_local global %struct.S zeroinitializer, align 4, !dbg !0
  call void @llvm.dbg.value(metadata i32* getelementptr inbounds (%struct.S, %struct.S* @g, i32 0, i32 0), metadata !20, metadata !DIExpression()), !dbg !22
  call void @llvm.dbg.value(metadata i32* getelementptr inbounds (%struct.S, %struct.S* @g, i32 0, i32 1), metadata !25, metadata !DIExpression()), !dbg !26
  call void @llvm.dbg.value(metadata i32* getelementptr inbounds (%struct.S, %struct.S* @g, i64 0, i32 1), metadata !25, metadata !DIExpression()), !dbg !26
  call void @work(i32* getelementptr inbounds (%struct.S, %struct.S* @g, i64 0, i32 1)) #4, !dbg !27
  call void @llvm.memcpy.p0i8.p0i8.i64(i8* align 4 bitcast (%struct.S* @g to i8*), i8* align 4 bitcast (%struct.S* @bar.l to i8*), i64 12, i1 false), !dbg !28
```

gives without this patch:

```
$ verify-uselistorder di.ll -debug 2>&1 | grep -A10 fail:
 - fail: user mismatch: ID = 0
 - LHS value = @g = common dso_local global %struct.S zeroinitializer, align 4, !dbg !0
   => use: op = 0, user-id = 0, user = i32* getelementptr inbounds (%struct.S, %struct.S* @g, i32 0, i32 0)
   => use: op = 0, user-id = 0, user = i32* getelementptr inbounds (%struct.S, %struct.S* @g, i32 0, i32 1)
   => use: op = 0, user-id = 27, user = i32* getelementptr inbounds (%struct.S, %struct.S* @g, i64 0, i32 1)
   => use: op = 0, user-id = 28, user = i8* bitcast (%struct.S* @g to i8*)
 - RHS value = @g = common dso_local global %struct.S zeroinitializer, align 4, !dbg !0
   => use: op = 0, user-id = 28, user = i8* bitcast (%struct.S* @g to i8*)
   => use: op = 0, user-id = 27, user = i32* getelementptr inbounds (%struct.S, %struct.S* @g, i64 0, i32 1)
   => use: op = 0, user-id = 0, user = i32* getelementptr inbounds (%struct.S, %struct.S* @g, i32 0, i32 1)
   => use: op = 0, user-id = 0, user = i32* getelementptr inbounds (%struct.S, %struct.S* @g, i32 0, i32 0)
```

So, unless I have misunderstood something, I assume that this will code be relevant even after that.

A run of verify-uselistorder on the attached test case does unfortunately not bail out for `@global_arr` (even if it were changed so that it does not trigger for `i64 0`), due to there only being one mapped user of the global (the GEP in the load). I can update the patch so that the verification fails for the global, and update the comment in the test case so that it just does not refer to the `i64 0`.


https://reviews.llvm.org/D53758