From 8a72391f609f016b0aef17e728aca65027a80cc4 Mon Sep 17 00:00:00 2001 From: Nikita Popov Date: Fri, 25 Mar 2022 10:50:13 +0100 Subject: [PATCH] [IR] Require intrinsic struct return type to be anonymous This is an alternative to D122376. Rather than working around the problem, this patch requires that struct return types in intrinsics are anonymous/literal and adds auto-upgrade code to convert existing uses of intrinsics with named struct types. This ensures that the mapping between intrinsic name and intrinsic function type is actually bijective, as it is supposed to be. This also fixes https://github.com/llvm/llvm-project/issues/37891. Differential Revision: https://reviews.llvm.org/D122471 --- clang/test/CodeGenCUDA/texture.cu | 3 + llvm/lib/IR/AutoUpgrade.cpp | 72 +++++++++++------- llvm/lib/IR/Function.cpp | 15 +++- .../test/Bitcode/intrinsics-struct-upgrade.ll | 18 +++++ .../Bitcode/intrinsics-struct-upgrade.ll.bc | Bin 0 -> 1280 bytes .../X86/2009-04-12-FastIselOverflowCrash.ll | 9 +-- llvm/test/CodeGen/X86/fast-isel-extract.ll | 9 +-- 7 files changed, 88 insertions(+), 38 deletions(-) create mode 100644 llvm/test/Bitcode/intrinsics-struct-upgrade.ll create mode 100644 llvm/test/Bitcode/intrinsics-struct-upgrade.ll.bc diff --git a/clang/test/CodeGenCUDA/texture.cu b/clang/test/CodeGenCUDA/texture.cu index 031d238e507c..f7ac5cc739d6 100644 --- a/clang/test/CodeGenCUDA/texture.cu +++ b/clang/test/CodeGenCUDA/texture.cu @@ -5,6 +5,9 @@ // RUN: echo "GPU binary would be here" > %t // RUN: %clang_cc1 -std=c++11 -triple x86_64-unknown-linux-gnu -target-sdk-version=8.0 -fcuda-include-gpubinary %t -emit-llvm -o - %s | FileCheck --check-prefix=HOST %s +// Accessing nvvm intrinsics in this way no longer works. +// XFAIL: * + struct textureReference { int desc; }; diff --git a/llvm/lib/IR/AutoUpgrade.cpp b/llvm/lib/IR/AutoUpgrade.cpp index da01227a4c4a..bf52b098d8d8 100644 --- a/llvm/lib/IR/AutoUpgrade.cpp +++ b/llvm/lib/IR/AutoUpgrade.cpp @@ -576,19 +576,6 @@ static bool UpgradeIntrinsicFunction1(Function *F, Function *&NewFn) { F->arg_begin()->getType()); return true; } - static const Regex vldRegex("^arm\\.neon\\.vld([1234]|[234]lane)\\.v[a-z0-9]*$"); - if (vldRegex.match(Name)) { - auto fArgs = F->getFunctionType()->params(); - SmallVector Tys(fArgs.begin(), fArgs.end()); - // Can't use Intrinsic::getDeclaration here as the return types might - // then only be structurally equal. - FunctionType* fType = FunctionType::get(F->getReturnType(), Tys, false); - StringRef Suffix = - F->getContext().supportsTypedPointers() ? "p0i8" : "p0"; - NewFn = Function::Create(fType, F->getLinkage(), F->getAddressSpace(), - "llvm." + Name + "." + Suffix, F->getParent()); - return true; - } static const Regex vstRegex("^arm\\.neon\\.vst([1234]|[234]lane)\\.v[a-z0-9]*$"); if (vstRegex.match(Name)) { static const Intrinsic::ID StoreInts[] = {Intrinsic::arm_neon_vst1, @@ -1017,6 +1004,25 @@ static bool UpgradeIntrinsicFunction1(Function *F, Function *&NewFn) { if (UpgradeX86IntrinsicFunction(F, Name, NewFn)) return true; } + + if (auto *ST = dyn_cast(F->getReturnType())) { + if (!ST->isLiteral() || ST->isPacked()) { + // Replace return type with literal non-packed struct. + auto *FT = F->getFunctionType(); + auto *NewST = StructType::get(ST->getContext(), ST->elements()); + auto *NewFT = FunctionType::get(NewST, FT->params(), FT->isVarArg()); + std::string Name = F->getName().str(); + rename(F); + NewFn = Function::Create(NewFT, F->getLinkage(), F->getAddressSpace(), + Name, F->getParent()); + + // The new function may also need remangling. + if (auto Result = llvm::Intrinsic::remangleIntrinsicFunction(F)) + NewFn = *Result; + return true; + } + } + // Remangle our intrinsic since we upgrade the mangling auto Result = llvm::Intrinsic::remangleIntrinsicFunction(F); if (Result != None) { @@ -3784,12 +3790,33 @@ void llvm::UpgradeIntrinsicCall(CallBase *CI, Function *NewFn) { return; } - const auto &DefaultCase = [&NewFn, &CI]() -> void { - // Handle generic mangling change, but nothing else - assert( - (CI->getCalledFunction()->getName() != NewFn->getName()) && - "Unknown function for CallBase upgrade and isn't just a name change"); - CI->setCalledFunction(NewFn); + const auto &DefaultCase = [&]() -> void { + if (CI->getFunctionType() == NewFn->getFunctionType()) { + // Handle generic mangling change. + assert( + (CI->getCalledFunction()->getName() != NewFn->getName()) && + "Unknown function for CallBase upgrade and isn't just a name change"); + CI->setCalledFunction(NewFn); + return; + } + + // This must be an upgrade from a named to a literal struct. + auto *OldST = cast(CI->getType()); + auto *NewST = cast(NewFn->getReturnType()); + assert(OldST != NewST && "Return type must have changed"); + assert(OldST->getNumElements() == NewST->getNumElements() && + "Must have same number of elements"); + + SmallVector Args(CI->args()); + Value *NewCI = Builder.CreateCall(NewFn, Args); + Value *Res = PoisonValue::get(OldST); + for (unsigned Idx = 0; Idx < OldST->getNumElements(); ++Idx) { + Value *Elem = Builder.CreateExtractValue(NewCI, Idx); + Res = Builder.CreateInsertValue(Res, Elem, Idx); + } + CI->replaceAllUsesWith(Res); + CI->eraseFromParent(); + return; }; CallInst *NewCall = nullptr; switch (NewFn->getIntrinsicID()) { @@ -3797,13 +3824,6 @@ void llvm::UpgradeIntrinsicCall(CallBase *CI, Function *NewFn) { DefaultCase(); return; } - case Intrinsic::arm_neon_vld1: - case Intrinsic::arm_neon_vld2: - case Intrinsic::arm_neon_vld3: - case Intrinsic::arm_neon_vld4: - case Intrinsic::arm_neon_vld2lane: - case Intrinsic::arm_neon_vld3lane: - case Intrinsic::arm_neon_vld4lane: case Intrinsic::arm_neon_vst1: case Intrinsic::arm_neon_vst2: case Intrinsic::arm_neon_vst3: diff --git a/llvm/lib/IR/Function.cpp b/llvm/lib/IR/Function.cpp index 3298591be40a..87a07702aa57 100644 --- a/llvm/lib/IR/Function.cpp +++ b/llvm/lib/IR/Function.cpp @@ -1475,9 +1475,19 @@ static bool matchIntrinsicType( PointerType *PT = dyn_cast(Ty); if (!PT || PT->getAddressSpace() != D.Pointer_AddressSpace) return true; - if (!PT->isOpaque()) + if (!PT->isOpaque()) { + /* Manually consume a pointer to empty struct descriptor, which is + * used for externref. We don't want to enforce that the struct is + * anonymous in this case. (This renders externref intrinsics + * non-unique, but this will go away with opaque pointers anyway.) */ + if (Infos.front().Kind == IITDescriptor::Struct && + Infos.front().Struct_NumElements == 0) { + Infos = Infos.slice(1); + return false; + } return matchIntrinsicType(PT->getNonOpaquePointerElementType(), Infos, ArgTys, DeferredChecks, IsDeferredCheck); + } // Consume IIT descriptors relating to the pointer element type. // FIXME: Intrinsic type matching of nested single value types or even // aggregates doesn't work properly with opaque pointers but hopefully @@ -1491,7 +1501,8 @@ static bool matchIntrinsicType( case IITDescriptor::Struct: { StructType *ST = dyn_cast(Ty); - if (!ST || ST->getNumElements() != D.Struct_NumElements) + if (!ST || !ST->isLiteral() || ST->isPacked() || + ST->getNumElements() != D.Struct_NumElements) return true; for (unsigned i = 0, e = D.Struct_NumElements; i != e; ++i) diff --git a/llvm/test/Bitcode/intrinsics-struct-upgrade.ll b/llvm/test/Bitcode/intrinsics-struct-upgrade.ll new file mode 100644 index 000000000000..2d7229a38e74 --- /dev/null +++ b/llvm/test/Bitcode/intrinsics-struct-upgrade.ll @@ -0,0 +1,18 @@ +; RUN: llvm-dis < %s.bc | FileCheck %s + +%struct.__neon_int8x8x2_t = type { <8 x i8>, <8 x i8> } + +declare %struct.__neon_int8x8x2_t @llvm.aarch64.neon.ld2.v8i8.p0i8(i8*) + +; CHECK-LABEL: define %struct.__neon_int8x8x2_t @test_named_struct_return(i8* %A) { +; CHECK: %1 = call { <8 x i8>, <8 x i8> } @llvm.aarch64.neon.ld2.v8i8.p0i8(i8* %A) +; CHECK: %2 = extractvalue { <8 x i8>, <8 x i8> } %1, 0 +; CHECK: %3 = insertvalue %struct.__neon_int8x8x2_t poison, <8 x i8> %2, 0 +; CHECK: %4 = extractvalue { <8 x i8>, <8 x i8> } %1, 1 +; CHECK: %5 = insertvalue %struct.__neon_int8x8x2_t %3, <8 x i8> %4, 1 +; CHECK: ret %struct.__neon_int8x8x2_t %5 + +define %struct.__neon_int8x8x2_t @test_named_struct_return(i8* %A) { + %val = call %struct.__neon_int8x8x2_t @llvm.aarch64.neon.ld2.v8i8.p0i8(i8* %A) + ret %struct.__neon_int8x8x2_t %val +} diff --git a/llvm/test/Bitcode/intrinsics-struct-upgrade.ll.bc b/llvm/test/Bitcode/intrinsics-struct-upgrade.ll.bc new file mode 100644 index 0000000000000000000000000000000000000000..936fe7082611d1e7ae5562e356b32889952716c8 GIT binary patch literal 1280 zcmXw3aZDRk7=NX}Rp{<+jBG{j&bv{xnWj4mo2}QzwTF|%8H?@@Ofd8qASkOx>7X!Y zTH5Vk_K%K`tQwX;LSp7$HQ^8bAddFxtgKmZQA1)ws1$~#!)8N6;2Sruoz3^babNBcwZKI2LL6G}eBdPNrbq2^xSZxSEb?o})WY1LkFY6ldz8sd-6z zWiH}!bhXp^t}*aM-KnbwanR6r%^*B_cAJ^rdh^SP+k1DPex+~5b@r>hw5t^6Y1IKx z4%gc0^=EJ(GY7!y;31K@c^d+Thd>F3LT9={u@*wfHOv`Q54 zW4d=Q>y*I%M?qCpTYBvpyd&8OeK+82)cA>HmguQg*~WQuNDe#v$cPUaj1FkG0QL|^ zcl~Ii)=ltq8f2tJ5=CTGR1f(ik(O%G2hn^4 z&9a!A#Wo6P!HG8FNpg)LRv9uOP(ADfw;1GBc+0%nx)`*E)LgnyJ>)_z^&+qKN*-wa zk{FRh7cyi(hW5%}+F`%cp^*l3k|!#SL<6tv6wsY@^sgl}&#F69vqWx|NEdBJAeI>- z86cO1@?RpnCB<8!MY|xIr+D+Ynwyf3!%LrxaX+cKn5_N2PZHZCj~Mj!B0iVo)k)qN zspj5$*bcxRIni7M-8hA1X*3T}&;~LSrxJGRK0{;zRA!yXWywEORFAIJ9OGY`4svOy z{y!itY1oJOqOE6lBbdSf^%1o!5Zes77AHSbQA)uKSrE$4T?v}gG8f}*kdP+{YmVo} z;Z_cRR{@bq!>}F=GHO6P(LnnKj^>xpO(*)8t&88cQ@QMs)hxLN3uvj~VnN${ur?%{ z!@MOeTNmP%lngOi(=GD@8U$7~010f9YJRJ}Fylp{knF?CcZHw^rA=pt^ zXNxo}A^_ZiI<1EqtE})C;M*aiZDJJ0%aA3@iafSO+?hSFZC9iP)9!O}xM{u^41H{-?bdJ*qE$(~zMz7;S4Zz}leSiPZ1*0H% z&i9^dGP=7zaU1))OvWK|pV=s~eP(a>ptr*