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

[Vedant Kumar via llvm-commits]

vsk added a subscriber: vsk.
vsk added a comment.

I have a few nits and two questions (first question inline).

Second question: I notice that we're changing objectsize(p, 1) to objectsize(p, 2) in the tests. Are existing users of objectsize(p, typ) affected by this, and if so, can we warn them?

thanks


================
Comment at: docs/LangRef.rst:4187
@@ +4186,3 @@
+  int main() {
+    void \*p = &bs[0].a.data[10];
+    void \*v = (char*)p + 1;
----------------
&b[0] ?

================
Comment at: lib/CodeGen/CodeGenPrepare.cpp:1364
@@ -1363,3 +1363,3 @@
       // Lower all uses of llvm.objectsize.*
-      bool Min = (cast<ConstantInt>(II->getArgOperand(1))->getZExtValue() == 1);
+      bool Min = cast<ConstantInt>(II->getArgOperand(1))->getZExtValue() & 2;
       Type *ReturnTy = CI->getType();
----------------
Could you use `int` for this? It's a nit, but the value isn't strictly binary.

================
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)
+
----------------
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?

================
Comment at: test/CodeGen/Generic/object-size-type.ll:37
@@ +36,2 @@
+declare void @keep_alive(i8*)
+declare i32 @llvm.objectsize.i32(i8*, i8)
----------------
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.


http://reviews.llvm.org/D12351