Incorrect IR involving the use of bzero

[Bharathi]

Summary

This issue is specific to the use of bzero in a conditional expression 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, the generation of Phi node appears to be incorrect. Looks like 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.

The proposed patch addresses this issue. The llvm test suite and spec runs are succcessful with the patch.

[rjmccall]

test/CodeGen/bzero.c

18 ↗

(On Diff #121923)

I think you could just add this to builtin.c, and please make the test directly call __builtin_bzero instead of relying on the recognition of bzero as a builtin. Also, please rename the test function to something like test_conditional_bzero.

[Bharathi]

Addressed review comments.

[rjmccall]

That looks great, thanks. Do you have commit privileges?

[Bharathi]

I do not have commit privileges. Could you (rjmccall) or someone please commit this patch on my behalf ?
Thanks.

[rjmccall]

r317776. Thanks for the contribution!

lib/CodeGen/CGBuiltin.cpp

RValue CodeGenFunction::EmitBuiltinExpr(const FunctionDecl *FD,
 
   case Builtin::BIbzero:
   case Builtin::BI__builtin_bzero: {
     Address Dest = EmitPointerWithAlignment(E->getArg(0));
     Value *SizeVal = EmitScalarExpr(E->getArg(1));
     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: {
     Address Dest = EmitPointerWithAlignment(E->getArg(0));
     Address Src = EmitPointerWithAlignment(E->getArg(1));
     Value *SizeVal = EmitScalarExpr(E->getArg(2));
     EmitNonNullArgCheck(RValue::get(Dest.getPointer()), E->getArg(0)->getType(),
                         E->getArg(0)->getExprLoc(), FD, 0);

test/CodeGen/builtins.c

void bar() {
   ld = __builtin_nansl("");      // CHECK: x86_fp80 0xK7FFFA000000000000000
   f = __builtin_nansf("0xAE98"); // CHECK: float    0x7FF015D300000000
   d = __builtin_nans("0xAE98");  // CHECK: double   0x7FF000000000AE98
   ld = __builtin_nansl("0xAE98");// CHECK: x86_fp80 0xK7FFF800000000000AE98
 
 }
 // CHECK: }
 
+// CHECK-LABEL: define void @test_conditional_bzero
+void test_conditional_bzero() {
+  char dst[20];
+  int _sz = 20, len = 20;
+  return (_sz
+          ? ((_sz >= len)
+              ? __builtin_bzero(dst, len)
+              : foo())
+          : __builtin_bzero(dst, len));
+  // CHECK: call void @llvm.memset
+  // CHECK: call void @llvm.memset
+  // CHECK-NOT: phi
+}
 
 // CHECK-LABEL: define void @test_float_builtins
 void test_float_builtins(float F, double D, long double LD) {
   volatile int res;
   res = __builtin_isinf(F);
   // CHECK:  call float @llvm.fabs.f32(float
   // CHECK:  fcmp oeq float {{.*}}, 0x7FF0000000000000