[cfe-dev] Bad IR involving the use of bzero

[Bharathi Seshadri 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 <rjmccall at apple.com> wrote:
>
>> On Nov 7, 2017, at 12:15 AM, Bharathi Seshadri via cfe-dev <cfe-dev at lists.llvm.org> 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
>> cfe-dev at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
>