[clang] 3637dc6 - [clang][CodeGen] Consistently return nullptr Values for void builtins and scalar initalization

Markus Böck via cfe-commits cfe-commits at lists.llvm.org
Mon Oct 24 12:41:18 PDT 2022


Author: Markus Böck
Date: 2022-10-24T21:41:13+02:00
New Revision: 3637dc601c4923721a69426187aa69dd6a71a053

URL: https://github.com/llvm/llvm-project/commit/3637dc601c4923721a69426187aa69dd6a71a053
DIFF: https://github.com/llvm/llvm-project/commit/3637dc601c4923721a69426187aa69dd6a71a053.diff

LOG: [clang][CodeGen] Consistently return nullptr Values for void builtins and scalar initalization

A common post condition of the various visitor functions in CodeGen is that instructions, that do not return any values, simply return a nullptr Value as a sentinel. This has not been the case however for calls to some builtins returning void, as well as for an initializer expression of the form `void()`. This would then lead to ICEs in CodeGen on code relying on nullptr being returned for void values, which is eg. the case for conditional expressions [0].
This patch fixes that by returning nullptr Values for intrinsics known not to return any values as well as for a scalar initializer returning void.

Fixes https://github.com/llvm/llvm-project/issues/53127

[0] https://github.com/llvm/llvm-project/blob/266ec801fb23f9f5f1d61ca9466e0805fbdb78a7/clang/lib/CodeGen/CGExprScalar.cpp#L4849-L4892

Differential Revision: https://reviews.llvm.org/D136548

Added: 
    clang/test/CodeGen/pr53127.cpp

Modified: 
    clang/lib/CodeGen/CGBuiltin.cpp
    clang/lib/CodeGen/CGExprScalar.cpp

Removed: 
    


################################################################################
diff  --git a/clang/lib/CodeGen/CGBuiltin.cpp b/clang/lib/CodeGen/CGBuiltin.cpp
index fbb6e85e37d6e..f69b1e80607f8 100644
--- a/clang/lib/CodeGen/CGBuiltin.cpp
+++ b/clang/lib/CodeGen/CGBuiltin.cpp
@@ -2518,11 +2518,11 @@ RValue CodeGenFunction::EmitBuiltinExpr(const GlobalDecl GD, unsigned BuiltinID,
   case Builtin::BI__builtin_va_start:
   case Builtin::BI__va_start:
   case Builtin::BI__builtin_va_end:
-    return RValue::get(
-        EmitVAStartEnd(BuiltinID == Builtin::BI__va_start
-                           ? EmitScalarExpr(E->getArg(0))
-                           : EmitVAListRef(E->getArg(0)).getPointer(),
-                       BuiltinID != Builtin::BI__builtin_va_end));
+    EmitVAStartEnd(BuiltinID == Builtin::BI__va_start
+                       ? EmitScalarExpr(E->getArg(0))
+                       : EmitVAListRef(E->getArg(0)).getPointer(),
+                   BuiltinID != Builtin::BI__builtin_va_end);
+    return RValue::get(nullptr);
   case Builtin::BI__builtin_va_copy: {
     Value *DstPtr = EmitVAListRef(E->getArg(0)).getPointer();
     Value *SrcPtr = EmitVAListRef(E->getArg(1)).getPointer();
@@ -2531,8 +2531,8 @@ RValue CodeGenFunction::EmitBuiltinExpr(const GlobalDecl GD, unsigned BuiltinID,
 
     DstPtr = Builder.CreateBitCast(DstPtr, Type);
     SrcPtr = Builder.CreateBitCast(SrcPtr, Type);
-    return RValue::get(Builder.CreateCall(CGM.getIntrinsic(Intrinsic::vacopy),
-                                          {DstPtr, SrcPtr}));
+    Builder.CreateCall(CGM.getIntrinsic(Intrinsic::vacopy), {DstPtr, SrcPtr});
+    return RValue::get(nullptr);
   }
   case Builtin::BI__builtin_abs:
   case Builtin::BI__builtin_labs:
@@ -2804,7 +2804,8 @@ RValue CodeGenFunction::EmitBuiltinExpr(const GlobalDecl GD, unsigned BuiltinID,
 
     Value *ArgValue = EmitScalarExpr(E->getArg(0));
     Function *FnAssume = CGM.getIntrinsic(Intrinsic::assume);
-    return RValue::get(Builder.CreateCall(FnAssume, ArgValue));
+    Builder.CreateCall(FnAssume, ArgValue);
+    return RValue::get(nullptr);
   }
   case Builtin::BI__arithmetic_fence: {
     // Create the builtin call if FastMath is selected, and the target
@@ -2925,7 +2926,8 @@ RValue CodeGenFunction::EmitBuiltinExpr(const GlobalDecl GD, unsigned BuiltinID,
       llvm::ConstantInt::get(Int32Ty, 3);
     Value *Data = llvm::ConstantInt::get(Int32Ty, 1);
     Function *F = CGM.getIntrinsic(Intrinsic::prefetch, Address->getType());
-    return RValue::get(Builder.CreateCall(F, {Address, RW, Locality, Data}));
+    Builder.CreateCall(F, {Address, RW, Locality, Data});
+    return RValue::get(nullptr);
   }
   case Builtin::BI__builtin_readcyclecounter: {
     Function *F = CGM.getIntrinsic(Intrinsic::readcyclecounter);
@@ -2938,9 +2940,11 @@ RValue CodeGenFunction::EmitBuiltinExpr(const GlobalDecl GD, unsigned BuiltinID,
     return RValue::get(Builder.CreateCall(F, {Begin, End}));
   }
   case Builtin::BI__builtin_trap:
-    return RValue::get(EmitTrapCall(Intrinsic::trap));
+    EmitTrapCall(Intrinsic::trap);
+    return RValue::get(nullptr);
   case Builtin::BI__debugbreak:
-    return RValue::get(EmitTrapCall(Intrinsic::debugtrap));
+    EmitTrapCall(Intrinsic::debugtrap);
+    return RValue::get(nullptr);
   case Builtin::BI__builtin_unreachable: {
     EmitUnreachable(E->getExprLoc());
 
@@ -3721,7 +3725,8 @@ RValue CodeGenFunction::EmitBuiltinExpr(const GlobalDecl GD, unsigned BuiltinID,
   }
   case Builtin::BI__builtin_unwind_init: {
     Function *F = CGM.getIntrinsic(Intrinsic::eh_unwind_init);
-    return RValue::get(Builder.CreateCall(F));
+    Builder.CreateCall(F);
+    return RValue::get(nullptr);
   }
   case Builtin::BI__builtin_extend_pointer: {
     // Extends a pointer to the size of an _Unwind_Word, which is
@@ -4482,8 +4487,9 @@ RValue CodeGenFunction::EmitBuiltinExpr(const GlobalDecl GD, unsigned BuiltinID,
     return EmitBuiltinNewDeleteCall(
         E->getCallee()->getType()->castAs<FunctionProtoType>(), E, false);
   case Builtin::BI__builtin_operator_delete:
-    return EmitBuiltinNewDeleteCall(
+    EmitBuiltinNewDeleteCall(
         E->getCallee()->getType()->castAs<FunctionProtoType>(), E, true);
+    return RValue::get(nullptr);
 
   case Builtin::BI__builtin_is_aligned:
     return EmitBuiltinIsAligned(E);
@@ -4653,7 +4659,8 @@ RValue CodeGenFunction::EmitBuiltinExpr(const GlobalDecl GD, unsigned BuiltinID,
   case Builtin::BI__builtin_coro_promise:
     return EmitCoroutineIntrinsic(E, Intrinsic::coro_promise);
   case Builtin::BI__builtin_coro_resume:
-    return EmitCoroutineIntrinsic(E, Intrinsic::coro_resume);
+    EmitCoroutineIntrinsic(E, Intrinsic::coro_resume);
+    return RValue::get(nullptr);
   case Builtin::BI__builtin_coro_frame:
     return EmitCoroutineIntrinsic(E, Intrinsic::coro_frame);
   case Builtin::BI__builtin_coro_noop:
@@ -4661,7 +4668,8 @@ RValue CodeGenFunction::EmitBuiltinExpr(const GlobalDecl GD, unsigned BuiltinID,
   case Builtin::BI__builtin_coro_free:
     return EmitCoroutineIntrinsic(E, Intrinsic::coro_free);
   case Builtin::BI__builtin_coro_destroy:
-    return EmitCoroutineIntrinsic(E, Intrinsic::coro_destroy);
+    EmitCoroutineIntrinsic(E, Intrinsic::coro_destroy);
+    return RValue::get(nullptr);
   case Builtin::BI__builtin_coro_done:
     return EmitCoroutineIntrinsic(E, Intrinsic::coro_done);
   case Builtin::BI__builtin_coro_alloc:
@@ -5094,7 +5102,8 @@ RValue CodeGenFunction::EmitBuiltinExpr(const GlobalDecl GD, unsigned BuiltinID,
     Value *Val = EmitScalarExpr(E->getArg(0));
     Address Address = EmitPointerWithAlignment(E->getArg(1));
     Value *HalfVal = Builder.CreateFPTrunc(Val, Builder.getHalfTy());
-    return RValue::get(Builder.CreateStore(HalfVal, Address));
+    Builder.CreateStore(HalfVal, Address);
+    return RValue::get(nullptr);
   }
   case Builtin::BI__builtin_load_half: {
     Address Address = EmitPointerWithAlignment(E->getArg(0));
@@ -5359,6 +5368,9 @@ RValue CodeGenFunction::EmitBuiltinExpr(const GlobalDecl GD, unsigned BuiltinID,
         V = Builder.CreateBitCast(V, RetTy);
     }
 
+    if (RetTy->isVoidTy())
+      return RValue::get(nullptr);
+
     return RValue::get(V);
   }
 
@@ -5376,6 +5388,8 @@ RValue CodeGenFunction::EmitBuiltinExpr(const GlobalDecl GD, unsigned BuiltinID,
   if (Value *V = EmitTargetBuiltinExpr(BuiltinID, E, ReturnValue)) {
     switch (EvalKind) {
     case TEK_Scalar:
+      if (V->getType()->isVoidTy())
+        return RValue::get(nullptr);
       return RValue::get(V);
     case TEK_Aggregate:
       return RValue::getAggregate(ReturnValue.getValue(),

diff  --git a/clang/lib/CodeGen/CGExprScalar.cpp b/clang/lib/CodeGen/CGExprScalar.cpp
index eaaeb052aaef4..7b1337b1be68c 100644
--- a/clang/lib/CodeGen/CGExprScalar.cpp
+++ b/clang/lib/CodeGen/CGExprScalar.cpp
@@ -467,6 +467,9 @@ class ScalarExprEmitter
     return llvm::ConstantInt::get(ConvertType(E->getType()), E->getValue());
   }
   Value *VisitCXXScalarValueInitExpr(const CXXScalarValueInitExpr *E) {
+    if (E->getType()->isVoidType())
+      return nullptr;
+
     return EmitNullValue(E->getType());
   }
   Value *VisitGNUNullExpr(const GNUNullExpr *E) {

diff  --git a/clang/test/CodeGen/pr53127.cpp b/clang/test/CodeGen/pr53127.cpp
new file mode 100644
index 0000000000000..97fe1291352d3
--- /dev/null
+++ b/clang/test/CodeGen/pr53127.cpp
@@ -0,0 +1,87 @@
+// NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py
+// RUN: %clang_cc1 -emit-llvm %s -o - -triple x86_64-unknown-linux-gnu | FileCheck %s
+
+bool e();
+
+void operator delete(void*);
+
+// CHECK-LABEL: @_Z1fPiz(
+// CHECK-NEXT:  entry:
+// CHECK-NEXT:    [[P_ADDR:%.*]] = alloca ptr, align 8
+// CHECK-NEXT:    [[L:%.*]] = alloca [1 x %struct.__va_list_tag], align 16
+// CHECK-NEXT:    [[L2:%.*]] = alloca [1 x %struct.__va_list_tag], align 16
+// CHECK-NEXT:    store ptr [[P:%.*]], ptr [[P_ADDR]], align 8
+// CHECK-NEXT:    [[CALL:%.*]] = call noundef zeroext i1 @_Z1ev()
+// CHECK-NEXT:    br i1 [[CALL]], label [[COND_TRUE:%.*]], label [[COND_FALSE:%.*]]
+// CHECK:       cond.true:
+// CHECK-NEXT:    call void @llvm.trap()
+// CHECK-NEXT:    br label [[COND_END:%.*]]
+// CHECK:       cond.false:
+// CHECK-NEXT:    br label [[COND_END]]
+// CHECK:       cond.end:
+// CHECK-NEXT:    [[CALL1:%.*]] = call noundef zeroext i1 @_Z1ev()
+// CHECK-NEXT:    br i1 [[CALL1]], label [[COND_TRUE2:%.*]], label [[COND_FALSE3:%.*]]
+// CHECK:       cond.true2:
+// CHECK-NEXT:    call void @llvm.debugtrap()
+// CHECK-NEXT:    br label [[COND_END4:%.*]]
+// CHECK:       cond.false3:
+// CHECK-NEXT:    br label [[COND_END4]]
+// CHECK:       cond.end4:
+// CHECK-NEXT:    [[CALL5:%.*]] = call noundef zeroext i1 @_Z1ev()
+// CHECK-NEXT:    [[TMP0:%.*]] = zext i1 [[CALL5]] to i64
+// CHECK-NEXT:    call void @llvm.assume(i1 true)
+// CHECK-NEXT:    [[CALL6:%.*]] = call noundef zeroext i1 @_Z1ev()
+// CHECK-NEXT:    br i1 [[CALL6]], label [[COND_TRUE7:%.*]], label [[COND_FALSE8:%.*]]
+// CHECK:       cond.true7:
+// CHECK-NEXT:    [[ARRAYDECAY:%.*]] = getelementptr inbounds [1 x %struct.__va_list_tag], ptr [[L]], i64 0, i64 0
+// CHECK-NEXT:    call void @llvm.va_start(ptr [[ARRAYDECAY]])
+// CHECK-NEXT:    br label [[COND_END9:%.*]]
+// CHECK:       cond.false8:
+// CHECK-NEXT:    br label [[COND_END9]]
+// CHECK:       cond.end9:
+// CHECK-NEXT:    [[CALL10:%.*]] = call noundef zeroext i1 @_Z1ev()
+// CHECK-NEXT:    br i1 [[CALL10]], label [[COND_TRUE11:%.*]], label [[COND_FALSE14:%.*]]
+// CHECK:       cond.true11:
+// CHECK-NEXT:    [[ARRAYDECAY12:%.*]] = getelementptr inbounds [1 x %struct.__va_list_tag], ptr [[L]], i64 0, i64 0
+// CHECK-NEXT:    [[ARRAYDECAY13:%.*]] = getelementptr inbounds [1 x %struct.__va_list_tag], ptr [[L2]], i64 0, i64 0
+// CHECK-NEXT:    call void @llvm.va_copy(ptr [[ARRAYDECAY12]], ptr [[ARRAYDECAY13]])
+// CHECK-NEXT:    br label [[COND_END15:%.*]]
+// CHECK:       cond.false14:
+// CHECK-NEXT:    br label [[COND_END15]]
+// CHECK:       cond.end15:
+// CHECK-NEXT:    [[CALL16:%.*]] = call noundef zeroext i1 @_Z1ev()
+// CHECK-NEXT:    br i1 [[CALL16]], label [[COND_TRUE17:%.*]], label [[COND_FALSE18:%.*]]
+// CHECK:       cond.true17:
+// CHECK-NEXT:    [[TMP1:%.*]] = load ptr, ptr [[P_ADDR]], align 8
+// CHECK-NEXT:    call void @llvm.prefetch.p0(ptr [[TMP1]], i32 0, i32 3, i32 1)
+// CHECK-NEXT:    br label [[COND_END19:%.*]]
+// CHECK:       cond.false18:
+// CHECK-NEXT:    br label [[COND_END19]]
+// CHECK:       cond.end19:
+// CHECK-NEXT:    [[CALL20:%.*]] = call noundef zeroext i1 @_Z1ev()
+// CHECK-NEXT:    br i1 [[CALL20]], label [[COND_TRUE21:%.*]], label [[COND_FALSE22:%.*]]
+// CHECK:       cond.true21:
+// CHECK-NEXT:    call void @llvm.eh.unwind.init()
+// CHECK-NEXT:    br label [[COND_END23:%.*]]
+// CHECK:       cond.false22:
+// CHECK-NEXT:    br label [[COND_END23]]
+// CHECK:       cond.end23:
+// CHECK-NEXT:    [[CALL24:%.*]] = call noundef zeroext i1 @_Z1ev()
+// CHECK-NEXT:    [[TMP2:%.*]] = zext i1 [[CALL24]] to i64
+// CHECK-NEXT:    [[TMP3:%.*]] = load ptr, ptr [[P_ADDR]], align 8
+// CHECK-NEXT:    call void @_ZdlPv(ptr noundef [[TMP3]]) #[[ATTR8:[0-9]+]]
+// CHECK-NEXT:    ret void
+//
+void f(int* p, ...)
+{
+  e() ? __builtin_trap() : void();
+  e() ? __builtin_debugtrap() : void();
+  e() ? __builtin_assume(true) : void();
+  __builtin_va_list l;
+  e() ? __builtin_va_start(l, p) : void();
+  __builtin_va_list l2;
+  e() ? __builtin_va_copy(l, l2) : void();
+  e() ? __builtin_prefetch(p) : void();
+  e() ? __builtin_unwind_init() : void();
+  e() ? __builtin_operator_delete(p) : void();
+}


        


More information about the cfe-commits mailing list