[deref] Handle byval/byref/sret/inalloc/preallocated arguments for deref-at-point semantics

[reames]

Summary

All of these are scoped allocations which remain dereferenceable during the lifetime of the callee.

[nikic]

llvm/lib/IR/Value.cpp

747

Why is the check for a sized type necessary? For an unsized type there would be no dereferenceable bytes, but I don't think it's something that really affects canBeFreed itself?

[reames]

llvm/lib/IR/Value.cpp

747

I don't know. It seemed like enough of a cornercase to preserve the conservative handling from elsewhere. I'm happy to remove it if you are confident doing so is correct.

[nikic]

llvm/lib/IR/Value.cpp

747

The check in the getPointerDereferenceableBytes() implementation is present because we're querying the actual type size there, which is going to assert for unsized types. I'm reasonably sure that we don't need the check here.

[reames]

address review comment

[nikic]

LGTM

llvm/lib/IR/Value.cpp

745

This is probably going to warn due to unused variable. hasPointeeInMemoryValueAttr() is the equivalent boolean query.

llvm/lib/IR/Value.cpp

 static bool canBeFreed(const Value *V) {
   assert(V->getType()->isPointerTy());
 
   // Cases that can simply never be deallocated
   // *) Constants aren't allocated per se, thus not deallocated either.
   if (isa<Constant>(V))
     return false;
 
+  // Handle byval/byref/sret/inalloca/preallocated arguments.  The storage
+  // lifetime is guaranteed to be longer than the callee's lifetime.
+  if (auto *A = dyn_cast<Argument>(V))
+    if (A->hasPointeeInMemoryValueAttr())
+      return false;
+
   const Function *F = nullptr;
   if (auto *I = dyn_cast<Instruction>(V))
     F = I->getFunction();
   if (auto *A = dyn_cast<Argument>(V))
     F = A->getParent();
 
   if (!F)
     return true;

llvm/test/Analysis/ValueTracking/memory-dereferenceable.ll

 %struct.A = type { [8 x i8], [5 x i8] }
 @globalstruct = external global %struct.A
 
 @globalptr.align1 = external global i8, align 1
 @globalptr.align16 = external global i8, align 16
 
 ; Loads from sret arguments
 ; CHECK-LABEL: 'test_sret'
-; GLOBAL: %sret_gep{{.*}}(aligned)
-; POINT-NOT: %sret_gep{{.*}}(aligned)
+; CHECK: %sret_gep{{.*}}(aligned)
 ; CHECK-NOT: %sret_gep_outside
 define void @test_sret(%struct.A* sret(%struct.A) %result) {
   %sret_gep = getelementptr inbounds %struct.A, %struct.A* %result, i64 0, i32 1, i64 2
   load i8, i8* %sret_gep
 
   %sret_gep_outside = getelementptr %struct.A, %struct.A* %result, i64 0, i32 1, i64 7
   load i8, i8* %sret_gep_outside
   ret void
define void @global_allocationsize() {
 
   %outside_allocation = getelementptr inbounds %struct.A, %struct.A* @globalstruct, i64 0, i32 1, i64 10
   %load12 = load i8, i8* %outside_allocation
   ret void
 }
 
 ; Loads from byval arguments
 ; CHECK-LABEL: 'byval'
-; GLOBAL: %i8_byval{{.*}}(aligned)
-; POINT-NOT: %i8_byval{{.*}}(aligned)
-; CHECK-NOT: %byval_cast
-; GLOBAL: %byval_gep{{.*}}(aligned)
-; POINT-NOT: %byval_gep{{.*}}(aligned)
-; FIXME: Should hold in the point semantics case too
+; CHECK: %i8_byval{{.*}}(aligned)
+; CHECK-NOT: %bad_byval_cast
+; CHECK: %byval_gep{{.*}}(aligned)
+; CHECK: %good_byval_cast{{.*}}(unaligned)
 define void @byval(i8* byval(i8) %i8_byval,
-                        %struct.A* byval(%struct.A) %A_byval) {
   call void @mayfree()
-  %i8_byval_load = load i8, i8* %i8_byval
+  load i8, i8* %i8_byval
 
-  %byval_cast = bitcast i8* %i8_byval to i32*
-  %bad_byval_load = load i32, i32* %byval_cast
+  %bad_byval_cast = bitcast i8* %i8_byval to i32*
+  load i32, i32* %bad_byval_cast
 
   %byval_gep = getelementptr inbounds %struct.A, %struct.A* %A_byval, i64 0, i32 1, i64 2
   load i8, i8* %byval_gep
+  %good_byval_cast = bitcast %struct.A* %A_byval to i32*
+  load i32, i32* %good_byval_cast
+
   ret void
 }
 
 ; CHECK-LABEL: 'f_0'
 ; GLOBAL: %ptr = inttoptr i32 %val to i32*, !dereferenceable !0
 ; POINT-NOT: %ptr = inttoptr i32 %val to i32*, !dereferenceable !0
 define i32 @f_0(i32 %val) {
   %ptr = inttoptr i32 %val to i32*, !dereferenceable !0