zero9178 created this revision.
zero9178 added reviewers: erichkeane, rjmccall, efriedma, asl, aaron.ballman.
Herald added a project: All.
zero9178 requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
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
-----
Note:
I opted to adjust these functions as an idealistic goal to be more consistent
with the rest of the codebase. An alternative implementation that would have
also fixed the above issue and would be less code is to simply handle it
within the visit function of the conditional expression, by simply checking if
it the type of the expression is void and returning bailing out by returning
nullptr there instead of creating the invalid phi node. If you prefer that
version or something of the sorts please let me know
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D136548
Files:
clang/lib/CodeGen/CGBuiltin.cpp
clang/lib/CodeGen/CGExprScalar.cpp
clang/test/CodeGen/pr53127.cpp
Index: clang/test/CodeGen/pr53127.cpp
===================================================================
--- clang/test/CodeGen/pr53127.cpp
+++ 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 - | 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();
+}
Index: clang/lib/CodeGen/CGExprScalar.cpp
===================================================================
--- clang/lib/CodeGen/CGExprScalar.cpp
+++ clang/lib/CodeGen/CGExprScalar.cpp
@@ -467,6 +467,9 @@
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) {
Index: clang/lib/CodeGen/CGBuiltin.cpp
===================================================================
--- clang/lib/CodeGen/CGBuiltin.cpp
+++ clang/lib/CodeGen/CGBuiltin.cpp
@@ -2518,11 +2518,11 @@
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,9 @@
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 +2805,8 @@
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 +2927,8 @@
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 +2941,11 @@
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 +3726,8 @@
}
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 +4488,9 @@
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 +4660,8 @@
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 +4669,8 @@
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 +5103,8 @@
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 +5369,9 @@
V = Builder.CreateBitCast(V, RetTy);
}
+ if (RetTy->isVoidTy())
+ return RValue::get(nullptr);
+
return RValue::get(V);
}
@@ -5376,6 +5389,8 @@
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(),
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
