Bad IR involving the use of bzero

classic Classic list List threaded Threaded
3 messages Options
Reply | Threaded
Open this post in threaded view
|

Bad IR involving the use of bzero

Alex Denisov via cfe-dev
I am seeing an issue that seems to be specific to the use of bzero as below.

bash-4.1$ cat test.c

typedef __SIZE_TYPE__ size_t;
void bzero(void*, size_t);
void foo(void);

void test_bzero() {
  char dst[20];
  int _sz = 20, len = 20;
  return (_sz
          ? ((_sz >= len)
             ? bzero(dst, len)
             : foo())
          : bzero(dst, len));
}

Compiling this test case results in the following error:
---------
Instruction does not dominate all uses!
  %arraydecay = getelementptr inbounds [20 x i8], [20 x i8]* %dst, i32 0, i32 0
  %cond = phi i8* [ %arraydecay, %cond.end ], [ %arraydecay3, %cond.false2 ]
fatal error: error in backend: Broken function found, compilation aborted!
---------

Inspecting the IR (see below), the generation of Phi node appears to
be incorrect. It seems to me
that the phi node (for merging dst) is rather unnecessary since bzero
returns void.
In EmitBuiltinExpr(), bzero is replaced by a call to memset, which
returns a pointer to the destination.
Here is a patch that fixes the issue. (The llvm test suite and spec
runs are succcessful with the patch).

Is this patch reasonable? If not, could some one advice where the
issue is likely to be?

Thanks,
Bharathi

-------------------------------------------------------------------------------

Index: CGBuiltin.cpp
===================================================================
--- CGBuiltin.cpp       (revision 316884)
+++ CGBuiltin.cpp       (working copy)
@@ -1431,7 +1431,7 @@
     EmitNonNullArgCheck(RValue::get(Dest.getPointer()),
E->getArg(0)->getType(),
                         E->getArg(0)->getExprLoc(), FD, 0);
     Builder.CreateMemSet(Dest, Builder.getInt8(0), SizeVal, false);
-    return RValue::get(Dest.getPointer());
+    return RValue::get(nullptr);
   }
   case Builtin::BImemcpy:
   case Builtin::BI__builtin_memcpy: {

The bad IR for the test case is :

define void @test_bzero() #0 {
entry:
  %dst = alloca [20 x i8], align 16
  %_sz = alloca i32, align 4
  %len = alloca i32, align 4
  store i32 20, i32* %_sz, align 4
  store i32 20, i32* %len, align 4
  %0 = load i32, i32* %_sz, align 4
  %tobool = icmp ne i32 %0, 0
  br i1 %tobool, label %cond.true, label %cond.false2

cond.true:                                        ; preds = %entry
  %1 = load i32, i32* %_sz, align 4
  %2 = load i32, i32* %len, align 4
  %cmp = icmp sge i32 %1, %2
  br i1 %cmp, label %cond.true1, label %cond.false

cond.true1:                                       ; preds = %cond.true
  %arraydecay = getelementptr inbounds [20 x i8], [20 x i8]* %dst, i32 0, i32 0
  %3 = load i32, i32* %len, align 4
  %conv = sext i32 %3 to i64
  call void @llvm.memset.p0i8.i64(i8* %arraydecay, i8 0, i64 %conv,
i32 16, i1 false)
  br label %cond.end

cond.false:                                       ; preds = %cond.true
  call void @foo()
  br label %cond.end

cond.end:                                         ; preds =
%cond.false, %cond.true1
  br label %cond.end5

cond.false2:                                      ; preds = %entry
  %arraydecay3 = getelementptr inbounds [20 x i8], [20 x i8]* %dst, i32 0, i32 0
  %4 = load i32, i32* %len, align 4
  %conv4 = sext i32 %4 to i64
  call void @llvm.memset.p0i8.i64(i8* %arraydecay3, i8 0, i64 %conv4,
i32 16, i1 false)
  br label %cond.end5

cond.end5:                                        ; preds =
%cond.false2, %cond.end
  %cond = phi i8* [ %arraydecay, %cond.end ], [ %arraydecay3, %cond.false2 ]
  ret void
}
Instruction does not dominate all uses!
  %arraydecay = getelementptr inbounds [20 x i8], [20 x i8]* %dst, i32 0, i32 0
  %cond = phi i8* [ %arraydecay, %cond.end ], [ %arraydecay3, %cond.false2 ]
fatal error: error in backend: Broken function found, compilation aborted!
_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: Bad IR involving the use of bzero

Alex Denisov via cfe-dev

> On Nov 7, 2017, at 12:15 AM, Bharathi Seshadri via cfe-dev <[hidden email]> wrote:
>
> I am seeing an issue that seems to be specific to the use of bzero as below.
>
> bash-4.1$ cat test.c
>
> typedef __SIZE_TYPE__ size_t;
> void bzero(void*, size_t);
> void foo(void);
>
> void test_bzero() {
>  char dst[20];
>  int _sz = 20, len = 20;
>  return (_sz
>          ? ((_sz >= len)
>             ? bzero(dst, len)
>             : foo())
>          : bzero(dst, len));
> }
>
> Compiling this test case results in the following error:
> ---------
> Instruction does not dominate all uses!
>  %arraydecay = getelementptr inbounds [20 x i8], [20 x i8]* %dst, i32 0, i32 0
>  %cond = phi i8* [ %arraydecay, %cond.end ], [ %arraydecay3, %cond.false2 ]
> fatal error: error in backend: Broken function found, compilation aborted!
> ---------
>
> Inspecting the IR (see below), the generation of Phi node appears to
> be incorrect. It seems to me
> that the phi node (for merging dst) is rather unnecessary since bzero
> returns void.
> In EmitBuiltinExpr(), bzero is replaced by a call to memset, which
> returns a pointer to the destination.
> Here is a patch that fixes the issue. (The llvm test suite and spec
> runs are succcessful with the patch).
>
> Is this patch reasonable? If not, could some one advice where the
> issue is likely to be?

The patch is reasonable.  "Returning" a value here is triggering code that is supposed
to be suppressed when the return value is void, but it only ends up mattering inside a
conditional operator.

If you wouldn't mind formalizing your test case above into a real lit test case, we can
commit your fix.

John.

>
> Thanks,
> Bharathi
>
> -------------------------------------------------------------------------------
>
> Index: CGBuiltin.cpp
> ===================================================================
> --- CGBuiltin.cpp       (revision 316884)
> +++ CGBuiltin.cpp       (working copy)
> @@ -1431,7 +1431,7 @@
>     EmitNonNullArgCheck(RValue::get(Dest.getPointer()),
> E->getArg(0)->getType(),
>                         E->getArg(0)->getExprLoc(), FD, 0);
>     Builder.CreateMemSet(Dest, Builder.getInt8(0), SizeVal, false);
> -    return RValue::get(Dest.getPointer());
> +    return RValue::get(nullptr);
>   }
>   case Builtin::BImemcpy:
>   case Builtin::BI__builtin_memcpy: {
>
> The bad IR for the test case is :
>
> define void @test_bzero() #0 {
> entry:
>  %dst = alloca [20 x i8], align 16
>  %_sz = alloca i32, align 4
>  %len = alloca i32, align 4
>  store i32 20, i32* %_sz, align 4
>  store i32 20, i32* %len, align 4
>  %0 = load i32, i32* %_sz, align 4
>  %tobool = icmp ne i32 %0, 0
>  br i1 %tobool, label %cond.true, label %cond.false2
>
> cond.true:                                        ; preds = %entry
>  %1 = load i32, i32* %_sz, align 4
>  %2 = load i32, i32* %len, align 4
>  %cmp = icmp sge i32 %1, %2
>  br i1 %cmp, label %cond.true1, label %cond.false
>
> cond.true1:                                       ; preds = %cond.true
>  %arraydecay = getelementptr inbounds [20 x i8], [20 x i8]* %dst, i32 0, i32 0
>  %3 = load i32, i32* %len, align 4
>  %conv = sext i32 %3 to i64
>  call void @llvm.memset.p0i8.i64(i8* %arraydecay, i8 0, i64 %conv,
> i32 16, i1 false)
>  br label %cond.end
>
> cond.false:                                       ; preds = %cond.true
>  call void @foo()
>  br label %cond.end
>
> cond.end:                                         ; preds =
> %cond.false, %cond.true1
>  br label %cond.end5
>
> cond.false2:                                      ; preds = %entry
>  %arraydecay3 = getelementptr inbounds [20 x i8], [20 x i8]* %dst, i32 0, i32 0
>  %4 = load i32, i32* %len, align 4
>  %conv4 = sext i32 %4 to i64
>  call void @llvm.memset.p0i8.i64(i8* %arraydecay3, i8 0, i64 %conv4,
> i32 16, i1 false)
>  br label %cond.end5
>
> cond.end5:                                        ; preds =
> %cond.false2, %cond.end
>  %cond = phi i8* [ %arraydecay, %cond.end ], [ %arraydecay3, %cond.false2 ]
>  ret void
> }
> Instruction does not dominate all uses!
>  %arraydecay = getelementptr inbounds [20 x i8], [20 x i8]* %dst, i32 0, i32 0
>  %cond = phi i8* [ %arraydecay, %cond.end ], [ %arraydecay3, %cond.false2 ]
> fatal error: error in backend: Broken function found, compilation aborted!
> _______________________________________________
> cfe-dev mailing list
> [hidden email]
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev

_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: Bad IR involving the use of bzero

Alex Denisov via cfe-dev
Thanks John.

I've posted the patch for review: https://reviews.llvm.org/D39746

Bharathi

On Mon, Nov 6, 2017 at 9:34 PM, John McCall <[hidden email]> wrote:

>
>> On Nov 7, 2017, at 12:15 AM, Bharathi Seshadri via cfe-dev <[hidden email]> wrote:
>>
>> I am seeing an issue that seems to be specific to the use of bzero as below.
>>
>> bash-4.1$ cat test.c
>>
>> typedef __SIZE_TYPE__ size_t;
>> void bzero(void*, size_t);
>> void foo(void);
>>
>> void test_bzero() {
>>  char dst[20];
>>  int _sz = 20, len = 20;
>>  return (_sz
>>          ? ((_sz >= len)
>>             ? bzero(dst, len)
>>             : foo())
>>          : bzero(dst, len));
>> }
>>
>> Compiling this test case results in the following error:
>> ---------
>> Instruction does not dominate all uses!
>>  %arraydecay = getelementptr inbounds [20 x i8], [20 x i8]* %dst, i32 0, i32 0
>>  %cond = phi i8* [ %arraydecay, %cond.end ], [ %arraydecay3, %cond.false2 ]
>> fatal error: error in backend: Broken function found, compilation aborted!
>> ---------
>>
>> Inspecting the IR (see below), the generation of Phi node appears to
>> be incorrect. It seems to me
>> that the phi node (for merging dst) is rather unnecessary since bzero
>> returns void.
>> In EmitBuiltinExpr(), bzero is replaced by a call to memset, which
>> returns a pointer to the destination.
>> Here is a patch that fixes the issue. (The llvm test suite and spec
>> runs are succcessful with the patch).
>>
>> Is this patch reasonable? If not, could some one advice where the
>> issue is likely to be?
>
> The patch is reasonable.  "Returning" a value here is triggering code that is supposed
> to be suppressed when the return value is void, but it only ends up mattering inside a
> conditional operator.
>
> If you wouldn't mind formalizing your test case above into a real lit test case, we can
> commit your fix.
>
> John.
>
>>
>> Thanks,
>> Bharathi
>>
>> -------------------------------------------------------------------------------
>>
>> Index: CGBuiltin.cpp
>> ===================================================================
>> --- CGBuiltin.cpp       (revision 316884)
>> +++ CGBuiltin.cpp       (working copy)
>> @@ -1431,7 +1431,7 @@
>>     EmitNonNullArgCheck(RValue::get(Dest.getPointer()),
>> E->getArg(0)->getType(),
>>                         E->getArg(0)->getExprLoc(), FD, 0);
>>     Builder.CreateMemSet(Dest, Builder.getInt8(0), SizeVal, false);
>> -    return RValue::get(Dest.getPointer());
>> +    return RValue::get(nullptr);
>>   }
>>   case Builtin::BImemcpy:
>>   case Builtin::BI__builtin_memcpy: {
>>
>> The bad IR for the test case is :
>>
>> define void @test_bzero() #0 {
>> entry:
>>  %dst = alloca [20 x i8], align 16
>>  %_sz = alloca i32, align 4
>>  %len = alloca i32, align 4
>>  store i32 20, i32* %_sz, align 4
>>  store i32 20, i32* %len, align 4
>>  %0 = load i32, i32* %_sz, align 4
>>  %tobool = icmp ne i32 %0, 0
>>  br i1 %tobool, label %cond.true, label %cond.false2
>>
>> cond.true:                                        ; preds = %entry
>>  %1 = load i32, i32* %_sz, align 4
>>  %2 = load i32, i32* %len, align 4
>>  %cmp = icmp sge i32 %1, %2
>>  br i1 %cmp, label %cond.true1, label %cond.false
>>
>> cond.true1:                                       ; preds = %cond.true
>>  %arraydecay = getelementptr inbounds [20 x i8], [20 x i8]* %dst, i32 0, i32 0
>>  %3 = load i32, i32* %len, align 4
>>  %conv = sext i32 %3 to i64
>>  call void @llvm.memset.p0i8.i64(i8* %arraydecay, i8 0, i64 %conv,
>> i32 16, i1 false)
>>  br label %cond.end
>>
>> cond.false:                                       ; preds = %cond.true
>>  call void @foo()
>>  br label %cond.end
>>
>> cond.end:                                         ; preds =
>> %cond.false, %cond.true1
>>  br label %cond.end5
>>
>> cond.false2:                                      ; preds = %entry
>>  %arraydecay3 = getelementptr inbounds [20 x i8], [20 x i8]* %dst, i32 0, i32 0
>>  %4 = load i32, i32* %len, align 4
>>  %conv4 = sext i32 %4 to i64
>>  call void @llvm.memset.p0i8.i64(i8* %arraydecay3, i8 0, i64 %conv4,
>> i32 16, i1 false)
>>  br label %cond.end5
>>
>> cond.end5:                                        ; preds =
>> %cond.false2, %cond.end
>>  %cond = phi i8* [ %arraydecay, %cond.end ], [ %arraydecay3, %cond.false2 ]
>>  ret void
>> }
>> Instruction does not dominate all uses!
>>  %arraydecay = getelementptr inbounds [20 x i8], [20 x i8]* %dst, i32 0, i32 0
>>  %cond = phi i8* [ %arraydecay, %cond.end ], [ %arraydecay3, %cond.false2 ]
>> fatal error: error in backend: Broken function found, compilation aborted!
>> _______________________________________________
>> cfe-dev mailing list
>> [hidden email]
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
>
_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev