diff --git a/llvm/lib/Transforms/IPO/WholeProgramDevirt.cpp b/llvm/lib/Transforms/IPO/WholeProgramDevirt.cpp --- a/llvm/lib/Transforms/IPO/WholeProgramDevirt.cpp +++ b/llvm/lib/Transforms/IPO/WholeProgramDevirt.cpp @@ -500,6 +500,10 @@ IntegerType *Int32Ty; IntegerType *Int64Ty; IntegerType *IntPtrTy; + /// Type for imported vtables. Because we can use multiple names to refer + /// to the same vtable, we use a sizeless array type to prevent optimizers + /// from assuming different names refer to different addresses. + ArrayType *VtableTy; bool RemarksEnabled; function_ref OREGetter; @@ -529,6 +533,7 @@ Int32Ty(Type::getInt32Ty(M.getContext())), Int64Ty(Type::getInt64Ty(M.getContext())), IntPtrTy(M.getDataLayout().getIntPtrType(M.getContext(), 0)), + VtableTy(ArrayType::get(Type::getInt8Ty(M.getContext()), 0)), RemarksEnabled(areRemarksEnabled()), OREGetter(OREGetter) { assert(!(ExportSummary && ImportSummary)); FunctionsToSkip.init(SkipFunctionNames); @@ -588,7 +593,7 @@ // This function is called during the import phase to create a reference to // the symbol definition created during the export phase. Constant *importGlobal(VTableSlot Slot, ArrayRef Args, - StringRef Name); + StringRef Name, Type *Ty); Constant *importConstant(VTableSlot Slot, ArrayRef Args, StringRef Name, IntegerType *IntTy, uint32_t Storage); @@ -1395,8 +1400,9 @@ void DevirtModule::exportGlobal(VTableSlot Slot, ArrayRef Args, StringRef Name, Constant *C) { - GlobalAlias *GA = GlobalAlias::create(Int8Ty, 0, GlobalValue::ExternalLinkage, - getGlobalName(Slot, Args, Name), C, &M); + GlobalAlias *GA = GlobalAlias::create( + cast(C->getType())->getElementType(), 0, + GlobalValue::ExternalLinkage, getGlobalName(Slot, Args, Name), C, &M); GA->setVisibility(GlobalValue::HiddenVisibility); } @@ -1414,8 +1420,8 @@ } Constant *DevirtModule::importGlobal(VTableSlot Slot, ArrayRef Args, - StringRef Name) { - Constant *C = M.getOrInsertGlobal(getGlobalName(Slot, Args, Name), Int8Ty); + StringRef Name, Type *Ty) { + Constant *C = M.getOrInsertGlobal(getGlobalName(Slot, Args, Name), Ty); auto *GV = dyn_cast(C); if (GV) GV->setVisibility(GlobalValue::HiddenVisibility); @@ -1428,7 +1434,7 @@ if (!shouldExportConstantsAsAbsoluteSymbols()) return ConstantInt::get(IntTy, Storage); - Constant *C = importGlobal(Slot, Args, Name); + Constant *C = importGlobal(Slot, Args, Name, Int8Ty); auto *GV = cast(C->stripPointerCasts()); C = ConstantExpr::getPtrToInt(C, IntTy); @@ -1457,8 +1463,8 @@ for (auto &&Call : CSInfo.CallSites) { IRBuilder<> B(Call.CS.getInstruction()); Value *Cmp = - B.CreateICmp(IsOne ? ICmpInst::ICMP_EQ : ICmpInst::ICMP_NE, - B.CreateBitCast(Call.VTable, Int8PtrTy), UniqueMemberAddr); + B.CreateICmp(IsOne ? ICmpInst::ICMP_EQ : ICmpInst::ICMP_NE, Call.VTable, + B.CreateBitCast(UniqueMemberAddr, Call.VTable->getType())); Cmp = B.CreateZExt(Cmp, Call.CS->getType()); Call.replaceAndErase("unique-ret-val", FnName, RemarksEnabled, OREGetter, Cmp); @@ -1873,7 +1879,7 @@ break; case WholeProgramDevirtResolution::ByArg::UniqueRetVal: { Constant *UniqueMemberAddr = - importGlobal(Slot, CSByConstantArg.first, "unique_member"); + importGlobal(Slot, CSByConstantArg.first, "unique_member", VtableTy); applyUniqueRetValOpt(CSByConstantArg.second, "", ResByArg.Info, UniqueMemberAddr); break; diff --git a/llvm/test/Transforms/WholeProgramDevirt/Inputs/unique-retval-same-vtable.yaml b/llvm/test/Transforms/WholeProgramDevirt/Inputs/unique-retval-same-vtable.yaml new file mode 100644 --- /dev/null +++ b/llvm/test/Transforms/WholeProgramDevirt/Inputs/unique-retval-same-vtable.yaml @@ -0,0 +1,17 @@ +--- +TypeIdMap: + _ZTS1C: + WPDRes: + 16: + Kind: Indir + ResByArg: + '': + Kind: UniqueRetVal + Info: 1 + 24: + Kind: Indir + ResByArg: + '': + Kind: UniqueRetVal + Info: 1 +... diff --git a/llvm/test/Transforms/WholeProgramDevirt/import.ll b/llvm/test/Transforms/WholeProgramDevirt/import.ll --- a/llvm/test/Transforms/WholeProgramDevirt/import.ll +++ b/llvm/test/Transforms/WholeProgramDevirt/import.ll @@ -85,8 +85,8 @@ cont: %fptr_casted = bitcast i8* %fptr to i1 (i8*, i32)* %result = call i1 %fptr_casted(i8* %obj, i32 3) - ; UNIQUE-RET-VAL0: icmp ne i8* %vtablei8, @__typeid_typeid2_8_3_unique_member - ; UNIQUE-RET-VAL1: icmp eq i8* %vtablei8, @__typeid_typeid2_8_3_unique_member + ; UNIQUE-RET-VAL0: icmp ne i8* %vtablei8, getelementptr inbounds ([0 x i8], [0 x i8]* @__typeid_typeid2_8_3_unique_member, i32 0, i32 0) + ; UNIQUE-RET-VAL1: icmp eq i8* %vtablei8, getelementptr inbounds ([0 x i8], [0 x i8]* @__typeid_typeid2_8_3_unique_member, i32 0, i32 0) ; VCP: [[VT2:%.*]] = bitcast {{.*}} to i8* ; VCP-X86: [[GEP2:%.*]] = getelementptr i8, i8* [[VT2]], i32 ptrtoint (i8* @__typeid_typeid2_8_3_byte to i32) ; VCP-ARM: [[GEP2:%.*]] = getelementptr i8, i8* [[VT2]], i32 43 diff --git a/llvm/test/Transforms/WholeProgramDevirt/unique-retval-same-vtable.ll b/llvm/test/Transforms/WholeProgramDevirt/unique-retval-same-vtable.ll new file mode 100644 --- /dev/null +++ b/llvm/test/Transforms/WholeProgramDevirt/unique-retval-same-vtable.ll @@ -0,0 +1,59 @@ +; Test for PR45393: Two virtual functions that return unique i1 values +; in the same vtable. Both calls are optimized to a comparison of +; this's vptr against the address of the vtable. When nesting these +; checks, LLVM would previously assume the nested check always fails, +; but that assumption does not hold if both checks refer to the same vtable. +; This tests checks that this case is handled correctly. +; +; RUN: opt -S -wholeprogramdevirt -wholeprogramdevirt-summary-action=import \ +; RUN: -wholeprogramdevirt-read-summary=%p/Inputs/unique-retval-same-vtable.yaml \ +; RUN: -O2 -o - %s | FileCheck %s +; +; Check that C::f() contains both possible return values. +; CHECK-LABEL: define {{.*}} @_ZNK1C1fEv +; CHECK-NOT: } +; CHECK: 20074028 +; CHECK-NOT: } +; CHECK: 1008434 + +target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128" +target triple = "x86_64-unknown-linux-gnu" + +%class.C = type { i32 (...)** } + +define hidden i32 @_ZNK1C1fEv(%class.C* %this) { +entry: + %0 = bitcast %class.C* %this to i1 (%class.C*)*** + %vtable = load i1 (%class.C*)**, i1 (%class.C*)*** %0 + %1 = bitcast i1 (%class.C*)** %vtable to i8* + %2 = tail call i1 @llvm.type.test(i8* %1, metadata !"_ZTS1C") + tail call void @llvm.assume(i1 %2) + %vfn = getelementptr inbounds i1 (%class.C*)*, i1 (%class.C*)** %vtable, i64 2 + %3 = load i1 (%class.C*)*, i1 (%class.C*)** %vfn + %call = tail call zeroext i1 %3(%class.C* %this) + br i1 %call, label %if.then, label %return + +if.then: + %vtable2 = load i1 (%class.C*)**, i1 (%class.C*)*** %0 + %4 = bitcast i1 (%class.C*)** %vtable2 to i8* + %5 = tail call i1 @llvm.type.test(i8* %4, metadata !"_ZTS1C") + tail call void @llvm.assume(i1 %5) + %vfn3 = getelementptr inbounds i1 (%class.C*)*, i1 (%class.C*)** %vtable2, i64 3 + %6 = load i1 (%class.C*)*, i1 (%class.C*)** %vfn3 + ; The method being called here and the method being called before + ; the branch above both return true in the same vtable and only that + ; vtable. Therefore, if this call is reached, we must select + ; 20074028. Earlier versions of LLVM mistakenly concluded that + ; this code *never* selects 200744028. + %call4 = tail call zeroext i1 %6(%class.C* nonnull %this) + %. = select i1 %call4, i32 20074028, i32 3007762 + br label %return + +return: + %retval.0 = phi i32 [ %., %if.then ], [ 1008434, %entry ] + ret i32 %retval.0 +} + +declare i1 @llvm.type.test(i8*, metadata) + +declare void @llvm.assume(i1) diff --git a/llvm/test/Transforms/WholeProgramDevirt/unique-retval.ll b/llvm/test/Transforms/WholeProgramDevirt/unique-retval.ll --- a/llvm/test/Transforms/WholeProgramDevirt/unique-retval.ll +++ b/llvm/test/Transforms/WholeProgramDevirt/unique-retval.ll @@ -20,15 +20,13 @@ define i1 @call1(i8* %obj) { %vtableptr = bitcast i8* %obj to [1 x i8*]** %vtable = load [1 x i8*]*, [1 x i8*]** %vtableptr - ; CHECK: {{.*}} = bitcast [1 x i8*]* {{.*}} to i8* - ; CHECK: [[VT1:%[^ ]*]] = bitcast [1 x i8*]* {{.*}} to i8* %vtablei8 = bitcast [1 x i8*]* %vtable to i8* %p = call i1 @llvm.type.test(i8* %vtablei8, metadata !"typeid1") call void @llvm.assume(i1 %p) %fptrptr = getelementptr [1 x i8*], [1 x i8*]* %vtable, i32 0, i32 0 %fptr = load i8*, i8** %fptrptr %fptr_casted = bitcast i8* %fptr to i1 (i8*)* - ; CHECK: [[RES1:%[^ ]*]] = icmp eq i8* [[VT1]], bitcast ([1 x i8*]* @vt3 to i8*) + ; CHECK: [[RES1:%[^ ]*]] = icmp eq [1 x i8*]* %vtable, @vt3 %result = call i1 %fptr_casted(i8* %obj) ; CHECK: ret i1 [[RES1]] ret i1 %result @@ -38,7 +36,6 @@ define i32 @call2(i8* %obj) { %vtableptr = bitcast i8* %obj to [1 x i8*]** %vtable = load [1 x i8*]*, [1 x i8*]** %vtableptr - ; CHECK: [[VT2:%[^ ]*]] = bitcast [1 x i8*]* {{.*}} to i8* %vtablei8 = bitcast [1 x i8*]* %vtable to i8* %p = call i1 @llvm.type.test(i8* %vtablei8, metadata !"typeid2") call void @llvm.assume(i1 %p) @@ -46,7 +43,7 @@ %fptr = load i8*, i8** %fptrptr ; Intentional type mismatch to test zero extend. %fptr_casted = bitcast i8* %fptr to i32 (i8*)* - ; CHECK: [[RES2:%[^ ]*]] = icmp ne i8* [[VT1]], bitcast ([1 x i8*]* @vt2 to i8*) + ; CHECK: [[RES2:%[^ ]*]] = icmp ne [1 x i8*]* %vtable, @vt2 %result = call i32 %fptr_casted(i8* %obj) ; CHECK: [[ZEXT2:%[^ ]*]] = zext i1 [[RES2]] to i32 ; CHECK: ret i32 [[ZEXT2:%[^ ]*]]