[InstCombine] Lower unknown @llvm.objectsize early.

[ab]

Summary

We already try to lower objectsize in InstCombine, but only if the
result is known. When it is unknown, the intrinsic calls would survive
the mid-level optimizers, to be lowered late, in CodeGenPrepare.

We can lower them in InstCombine as well, to 0 or -1, depending on the
min argument.
One could argue this is a bit early to do the lowering, since the size
might be made apparent by later optimizations. In practice however,
the majority of cases has a never-known objectsize, and in a lot of
the remaining few, the size is immediately apparent (say from a
global value, or an alloca).
Always keeping the intrinsic call intact prevents optimizations, and
makes memcpyopt useless when libcall fortification is enabled (the
default on a few major targets).

There are a few alternatives to doing it here; all seem overkill:

• teach the libcall simplifier to look through unknown @llvm.objectsize, so it's able to replace the fortified version of a libcall with the non-fortified one if the size is unknown.
• make a dedicated pass that lowers the intrinsic, even when the size is unknown, run it before memcpyopt & friends.
• in memcpyopt, start by running the libcall simplifier, to lower @llvm.objectsize, even when the size is unknown.

Also, Nuno proposes a related improvement to the associated
APIs (MemoryBuiltins.h): "[the analyzer] could be improved to
produce an interval instead. If we know that the minimum
objectsize is larger than the written size, then the check could
go away, even if we don't know the exact size of the buffer."

[nicholas]

Ahmed Bougacha wrote:

We already try to lower objectsize in InstCombine, but only if the
result is known. When it is unknown, the intrinsic calls would survive
the mid-level optimizers, to be lowered late, in CodeGenPrepare.

We can lower them in InstCombine as well, to 0 or -1, depending on the
min argument.
One could argue this is a bit early to do the lowering, since the size
might be made apparent by later optimizations.

Yep.

In practice however,

the majority of cases has a never-known objectsize, and in a lot of
the remaining few, the size is immediately apparent (say from a
global value, or an alloca).

Sure I'd expect that, but that isn't really a problem on its own.

Always keeping the intrinsic call intact prevents optimizations, and
makes memcpyopt useless when libcall fortification is enabled (the
default on a few major targets).

Can we fix that instead? Not waiting as long as possible to lower
unknown llvm.objectsize just can't be right.

There are a few alternatives to doing it here; all seem overkill:

• teach the libcall simplifier to look through unknown @llvm.objectsize, so it's able to replace the fortified version of a libcall with the non-fortified one if the size is unknown.
• make a dedicated pass that lowers the intrinsic, even when the size is unknown, run it before memcpyopt& friends.
• in memcpyopt, start by running the libcall simplifier, to lower @llvm.objectsize, even when the size is unknown.

The first of those sounds best to me.

Also, Nuno proposes a related improvement to the associated
APIs (MemoryBuiltins.h): "[the analyzer] could be improved to
produce an interval instead. If we know that the minimum
objectsize is larger than the written size, then the check could
go away, even if we don't know the exact size of the buffer."

http://reviews.llvm.org/D7129

Files:

lib/Transforms/InstCombine/InstCombineCalls.cpp
test/Transforms/InstCombine/objsize.ll
test/Transforms/InstCombine/stpcpy_chk-1.ll
test/Transforms/InstCombine/strcpy_chk-1.ll

EMAIL PREFERENCES

http://reviews.llvm.org/settings/panel/emailpreferences/

---

llvm-commits mailing list
[e-mail]
http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits

lib/Transforms/InstCombine/InstCombineCalls.cpp

if (MemIntrinsic *MI = dyn_cast<MemIntrinsic>(II)) {
     }
 
     if (Changed) return II;
   }
 
   switch (II->getIntrinsicID()) {
   default: break;
   case Intrinsic::objectsize: {
+    bool MinTrue = CI.getArgOperand(1) == ConstantInt::getTrue(CI.getContext());
+    Constant *SizeC = nullptr;
     uint64_t Size;
+    // Try to lower objectsize.  If the size is unknown, lower it to 0 or -1,
+    // depending on the min argument.
+    // One could argue this is a bit early to do the lowering, since the size
+    // might be made apparent by later optimizations.  In practice however,
+    // the overwhelming majority of cases has a never-known objectsize, and in
+    // a lot of the remaining few, the size is immediately apparent (say from a
+    // global value, or an alloca).
+    // Keeping the intrinsic call intact mostly prevents optimizations.
     if (getObjectSize(II->getArgOperand(0), Size, DL, TLI))
-      return ReplaceInstUsesWith(CI, ConstantInt::get(CI.getType(), Size));
-    return nullptr;
+      SizeC = ConstantInt::get(CI.getType(), Size);
+    else
+      SizeC = ConstantInt::get(CI.getType(), MinTrue ? 0 : -1);
+    return ReplaceInstUsesWith(CI, SizeC);
   }
   case Intrinsic::bswap: {
     Value *IIOperand = II->getArgOperand(0);
     Value *X = nullptr;
 
     // bswap(bswap(x)) -> x
     if (match(IIOperand, m_BSwap(m_Value(X))))
         return ReplaceInstUsesWith(CI, X);

test/Transforms/InstCombine/objsize.ll

; CHECK-NEXT: ret i32 0
   %1 = call i32 @llvm.objectsize.i32.p0i8(i8* getelementptr ([60 x i8]* @a, i32 1, i32 0), i1 false)
   ret i32 %1
 }
 
 @window = external global [0 x i8]
 
 define i1 @baz() nounwind {
 ; CHECK-LABEL: @baz(
-; CHECK-NEXT: objectsize
+; CHECK-NEXT: ret i1 true
   %1 = tail call i32 @llvm.objectsize.i32.p0i8(i8* getelementptr inbounds ([0 x i8]* @window, i32 0, i32 0), i1 false)
   %2 = icmp eq i32 %1, -1
   ret i1 %2
 }
 
 define void @test1(i8* %q, i32 %x) nounwind noinline {
 ; CHECK-LABEL: @test1(
-; CHECK: objectsize.i32.p0i8
+; CHECK: br i1 true, label %"47", label %"46"
 entry:
   %0 = call i32 @llvm.objectsize.i32.p0i8(i8* getelementptr inbounds ([0 x i8]* @window, i32 0, i32 10), i1 false) ; <i64> [#uses=1]
   %1 = icmp eq i32 %0, -1                         ; <i1> [#uses=1]
   br i1 %1, label %"47", label %"46"
 
 "46":                                             ; preds = %entry
   unreachable
 
entry:
   br i1 %cond, label %return, label %xpto
 
 xpto:
   %select = select i1 %bool, i8* %select, i8* %a
   %select2 = select i1 %bool, i8* %a, i8* %select2
   %0 = tail call i32 @llvm.objectsize.i32.p0i8(i8* %select, i1 true)
   %1 = tail call i32 @llvm.objectsize.i32.p0i8(i8* %select2, i1 true)
   %2 = add i32 %0, %1
-; CHECK: ret i32 undef
+; CHECK: ret i32 0
   ret i32 %2
 
 return:
   ret i32 42
 }
 
 ; CHECK-LABEL: @PR13621(
 define i32 @PR13621(i1 %bool) nounwind {
 entry:
   %cond = or i1 %bool, true
   br i1 %cond, label %return, label %xpto
 
 ; technically reachable, but this malformed IR may appear as a result of constant propagation
 xpto:
   %gep2 = getelementptr i8* %gep, i32 1
   %gep = getelementptr i8* %gep2, i32 1
   %o = call i32 @llvm.objectsize.i32.p0i8(i8* %gep, i1 true)
-; CHECK: ret i32 undef
+; CHECK: ret i32 0
   ret i32 %o
 
 return:
   ret i32 7
 }
 
 @globalalias = internal alias [60 x i8]* @a
 
 ; CHECK-LABEL: @test18(
 ; CHECK-NEXT: ret i32 60
 define i32 @test18() {
   %bc = bitcast [60 x i8]* @globalalias to i8*
   %1 = call i32 @llvm.objectsize.i32.p0i8(i8* %bc, i1 false)
   ret i32 %1
 }
 
 @globalalias2 = weak alias [60 x i8]* @a
 
 ; CHECK-LABEL: @test19(
-; CHECK: llvm.objectsize
+; CHECK: ret i32 -1
 define i32 @test19() {
   %bc = bitcast [60 x i8]* @globalalias2 to i8*
   %1 = call i32 @llvm.objectsize.i32.p0i8(i8* %bc, i1 false)
   ret i32 %1
 }
 

test/Transforms/InstCombine/stpcpy_chk-1.ll

 
 ; Check case where the string length is not constant.
 
 define i8* @test_simplify5() {
 ; CHECK-LABEL: @test_simplify5(
   %dst = getelementptr inbounds [60 x i8]* @a, i32 0, i32 0
   %src = getelementptr inbounds [12 x i8]* @.str, i32 0, i32 0
 
-; CHECK: @__memcpy_chk
+; CHECK: call void @llvm.memcpy.p0i8.p0i8.i32
   %len = call i32 @llvm.objectsize.i32.p0i8(i8* %dst, i1 false)
   %ret = call i8* @__stpcpy_chk(i8* %dst, i8* %src, i32 %len)
 ; CHECK: ret i8* getelementptr inbounds ([60 x i8]* @a, i32 0, i32 11)
   ret i8* %ret
 }
 
 ; Check case where the source and destination are the same.
 

test/Transforms/InstCombine/strcpy_chk-1.ll

 
 ; Check case where the string length is not constant.
 
 define void @test_simplify5() {
 ; CHECK-LABEL: @test_simplify5(
   %dst = getelementptr inbounds [60 x i8]* @a, i32 0, i32 0
   %src = getelementptr inbounds [12 x i8]* @.str, i32 0, i32 0
 
-; CHECK: @__memcpy_chk
+; CHECK: call void @llvm.memcpy.p0i8.p0i8.i32
   %len = call i32 @llvm.objectsize.i32.p0i8(i8* %dst, i1 false)
   call i8* @__strcpy_chk(i8* %dst, i8* %src, i32 %len)
   ret void
 }
 
 ; Check case where the source and destination are the same.
 
 define i8* @test_simplify6() {