[PATCH] D12351: Changing @llvm.objectsize(i8*, i1) to @llvm.objectsize(i8*, i8)

[George Burgess IV via llvm-commits]

george.burgess.iv added inline comments.

================
Comment at: test/CodeGen/Generic/object-size-type.ll:26
@@ +25,3 @@
+  %4 = call i32 @llvm.objectsize.i32(i8* %i, i8 3)
+  call void @consume(i8 3, i32 %4)
+
----------------
vsk wrote:
> Sorry, I may be missing something basic but I don't understand this result. Type=3 should mean find a lower bound of the size from %i to the closest enclosing array. Shouldn't that also be 4?
> 
> In your doc, I see: "If it can’t find these tags on potential sources of Ptr, it answers conservatively". Is the problem here that we don't have metadata on the gep?
> Type=3 should mean find a lower bound of the size from %i to the closest enclosing array
Not exactly. Type=3 finds the lower bound of the size of the closest source-level subobject. If this subobject is part of an array, then it will get the # of bytes remaining in the array. This exception does not apply for containers of the subobject.

> Is the problem here that we don't have metadata on the gep?
Yup! The issue is a combination of two things:

- We're asking for a lower-bound, so higher-than-optimal answers are not allowed (the opposite is true with Type=1)
- We have no clue what the source-level types are, and are being asked for the size of a subobject. Consider:

```
struct Foo { char c; };
struct { struct Foo[8] fs; } b;
__builtin_object_size(&b.fs[4].c, 3); // Pretend this slipped through from clang -- currently clang can answer this correctly.
```

In this case, the optimal result of BOS is 1, because `sizeof(b.fs[4].c) == 1`. To your point, 4 would be correct if we changed the BOS call to:

```
__builtin_object_size(&b.fs[4], 3);
```

================
Comment at: test/CodeGen/Generic/object-size-type.ll:37
@@ +36,2 @@
+declare void @keep_alive(i8*)
+declare i32 @llvm.objectsize.i32(i8*, i8)
----------------
vsk wrote:
> Is it possible to add CHECKs here for the GEP metadata you expect? It'd be good to have at least one check for the 4 different types.
I can add checks and make the test an XFAIL if that would be preferable. This patch just brings us to using an `i8` for Type. Patch for handling the metadata appropriately is in the works. :)


http://reviews.llvm.org/D12351