From 532bb80f7fd22bbc9896d9233f2a9ed79071716c Mon Sep 17 00:00:00 2001 From: Augie Fackler Date: Tue, 7 Sep 2021 14:49:03 -0400 Subject: [PATCH 1/3] RustWrapper: avoid deleted unclear attribute methods These were deleted in https://reviews.llvm.org/D108614, and in C++ I definitely see the argument for their removal. I didn't try and propagate the changes up into higher layers of rustc in this change because my initial goal was to get rustc working against LLVM HEAD promptly, but I'm happy to follow up with some refactoring to make the API on the Rust side match the LLVM API more directly (though the way the enum works in Rust makes the API less scary IMO). r? @nagisa cc @nikic --- .../rustc_llvm/llvm-wrapper/RustWrapper.cpp | 88 +++++++++++++++---- 1 file changed, 70 insertions(+), 18 deletions(-) diff --git a/compiler/rustc_llvm/llvm-wrapper/RustWrapper.cpp b/compiler/rustc_llvm/llvm-wrapper/RustWrapper.cpp index 4f07a0c67c13f..b3143f14d29cc 100644 --- a/compiler/rustc_llvm/llvm-wrapper/RustWrapper.cpp +++ b/compiler/rustc_llvm/llvm-wrapper/RustWrapper.cpp @@ -203,20 +203,59 @@ static Attribute::AttrKind fromRust(LLVMRustAttribute Kind) { report_fatal_error("bad AttributeKind"); } +template static inline void AddAttribute(T *t, unsigned Index, Attribute Attr) { +#if LLVM_VERSION_LT(14, 0) + t->addAttribute(Index, Attr); +#else + // TODO(durin42): we should probably surface the explicit functions to Rust + // instead of this switch statement? + switch (Index) { + case AttributeList::ReturnIndex: + t->addRetAttr(Attr); + break; + case AttributeList::FunctionIndex: + t->addFnAttr(Attr); + break; + default: + t->addParamAttr(Index-AttributeList::FirstArgIndex, Attr); + } +#endif +} + extern "C" void LLVMRustAddCallSiteAttribute(LLVMValueRef Instr, unsigned Index, LLVMRustAttribute RustAttr) { CallBase *Call = unwrap(Instr); Attribute Attr = Attribute::get(Call->getContext(), fromRust(RustAttr)); - Call->addAttribute(Index, Attr); + AddAttribute(Call, Index, Attr); } extern "C" void LLVMRustAddCallSiteAttrString(LLVMValueRef Instr, unsigned Index, const char *Name) { CallBase *Call = unwrap(Instr); Attribute Attr = Attribute::get(Call->getContext(), Name); - Call->addAttribute(Index, Attr); + AddAttribute(Call, Index, Attr); } +static inline void AddCallAttributes(CallBase *Call, unsigned Index, const AttrBuilder& B) { + AttributeList Attrs = Call->getAttributes(); +#if LLVM_VERSION_LT(14, 0) + Attrs = Attrs.addAttributes(Call->getContext(), Index, B); +#else + // TODO(durin42): we should probably surface the explicit functions to Rust + // instead of this switch statement? + switch (Index) { + case AttributeList::ReturnIndex: + Attrs = Attrs.addRetAttributes(Call->getContext(), B); + break; + case AttributeList::FunctionIndex: + Attrs = Attrs.addFnAttributes(Call->getContext(), B); + break; + default: + Attrs = Attrs.addParamAttributes(Call->getContext(), Index-AttributeList::FirstArgIndex, B); + } +#endif + Call->setAttributes(Attrs); +} extern "C" void LLVMRustAddAlignmentCallSiteAttr(LLVMValueRef Instr, unsigned Index, @@ -224,8 +263,7 @@ extern "C" void LLVMRustAddAlignmentCallSiteAttr(LLVMValueRef Instr, CallBase *Call = unwrap(Instr); AttrBuilder B; B.addAlignmentAttr(Bytes); - Call->setAttributes(Call->getAttributes().addAttributes( - Call->getContext(), Index, B)); + AddCallAttributes(Call, Index, B); } extern "C" void LLVMRustAddDereferenceableCallSiteAttr(LLVMValueRef Instr, @@ -234,8 +272,7 @@ extern "C" void LLVMRustAddDereferenceableCallSiteAttr(LLVMValueRef Instr, CallBase *Call = unwrap(Instr); AttrBuilder B; B.addDereferenceableAttr(Bytes); - Call->setAttributes(Call->getAttributes().addAttributes( - Call->getContext(), Index, B)); + AddCallAttributes(Call, Index, B); } extern "C" void LLVMRustAddDereferenceableOrNullCallSiteAttr(LLVMValueRef Instr, @@ -244,15 +281,14 @@ extern "C" void LLVMRustAddDereferenceableOrNullCallSiteAttr(LLVMValueRef Instr, CallBase *Call = unwrap(Instr); AttrBuilder B; B.addDereferenceableOrNullAttr(Bytes); - Call->setAttributes(Call->getAttributes().addAttributes( - Call->getContext(), Index, B)); + AddCallAttributes(Call, Index, B); } extern "C" void LLVMRustAddByValCallSiteAttr(LLVMValueRef Instr, unsigned Index, LLVMTypeRef Ty) { CallBase *Call = unwrap(Instr); Attribute Attr = Attribute::getWithByValType(Call->getContext(), unwrap(Ty)); - Call->addAttribute(Index, Attr); + AddAttribute(Call, Index, Attr); } extern "C" void LLVMRustAddStructRetCallSiteAttr(LLVMValueRef Instr, unsigned Index, @@ -263,28 +299,28 @@ extern "C" void LLVMRustAddStructRetCallSiteAttr(LLVMValueRef Instr, unsigned In #else Attribute Attr = Attribute::get(Call->getContext(), Attribute::StructRet); #endif - Call->addAttribute(Index, Attr); + AddAttribute(Call, Index, Attr); } extern "C" void LLVMRustAddFunctionAttribute(LLVMValueRef Fn, unsigned Index, LLVMRustAttribute RustAttr) { Function *A = unwrap(Fn); Attribute Attr = Attribute::get(A->getContext(), fromRust(RustAttr)); - A->addAttribute(Index, Attr); + AddAttribute(A, Index, Attr); } extern "C" void LLVMRustAddAlignmentAttr(LLVMValueRef Fn, unsigned Index, uint32_t Bytes) { Function *A = unwrap(Fn); - A->addAttribute(Index, Attribute::getWithAlignment( + AddAttribute(A, Index, Attribute::getWithAlignment( A->getContext(), llvm::Align(Bytes))); } extern "C" void LLVMRustAddDereferenceableAttr(LLVMValueRef Fn, unsigned Index, uint64_t Bytes) { Function *A = unwrap(Fn); - A->addAttribute(Index, Attribute::getWithDereferenceableBytes(A->getContext(), + AddAttribute(A, Index, Attribute::getWithDereferenceableBytes(A->getContext(), Bytes)); } @@ -292,7 +328,7 @@ extern "C" void LLVMRustAddDereferenceableOrNullAttr(LLVMValueRef Fn, unsigned Index, uint64_t Bytes) { Function *A = unwrap(Fn); - A->addAttribute(Index, Attribute::getWithDereferenceableOrNullBytes( + AddAttribute(A, Index, Attribute::getWithDereferenceableOrNullBytes( A->getContext(), Bytes)); } @@ -300,7 +336,7 @@ extern "C" void LLVMRustAddByValAttr(LLVMValueRef Fn, unsigned Index, LLVMTypeRef Ty) { Function *F = unwrap(Fn); Attribute Attr = Attribute::getWithByValType(F->getContext(), unwrap(Ty)); - F->addAttribute(Index, Attr); + AddAttribute(F, Index, Attr); } extern "C" void LLVMRustAddStructRetAttr(LLVMValueRef Fn, unsigned Index, @@ -311,7 +347,7 @@ extern "C" void LLVMRustAddStructRetAttr(LLVMValueRef Fn, unsigned Index, #else Attribute Attr = Attribute::get(F->getContext(), Attribute::StructRet); #endif - F->addAttribute(Index, Attr); + AddAttribute(F, Index, Attr); } extern "C" void LLVMRustAddFunctionAttrStringValue(LLVMValueRef Fn, @@ -319,7 +355,7 @@ extern "C" void LLVMRustAddFunctionAttrStringValue(LLVMValueRef Fn, const char *Name, const char *Value) { Function *F = unwrap(Fn); - F->addAttribute(Index, Attribute::get( + AddAttribute(F, Index, Attribute::get( F->getContext(), StringRef(Name), StringRef(Value))); } @@ -330,7 +366,23 @@ extern "C" void LLVMRustRemoveFunctionAttributes(LLVMValueRef Fn, Attribute Attr = Attribute::get(F->getContext(), fromRust(RustAttr)); AttrBuilder B(Attr); auto PAL = F->getAttributes(); - auto PALNew = PAL.removeAttributes(F->getContext(), Index, B); + AttributeList PALNew; +#if LLVM_VERSION_LT(14, 0) + PALNew = PAL.removeAttributes(F->getContext(), Index, B); +#else + // TODO(durin42): we should probably surface the explicit functions to Rust + // instead of this switch statement? + switch (Index) { + case AttributeList::ReturnIndex: + PALNew = PAL.removeRetAttributes(F->getContext(), B); + break; + case AttributeList::FunctionIndex: + PALNew = PAL.removeFnAttributes(F->getContext(), B); + break; + default: + PALNew = PAL.removeParamAttributes(F->getContext(), Index-AttributeList::FirstArgIndex, B); + } +#endif F->setAttributes(PALNew); } From 484b79b950b1077d1bbfe6c4edf3bfe070d820b4 Mon Sep 17 00:00:00 2001 From: Augie Fackler Date: Tue, 7 Sep 2021 16:15:02 -0400 Subject: [PATCH 2/3] RustWrapper: just use the *AtIndex funcs directly Otherwise we're kind of reimplementing the inverse of the well-named methods, and that's not a direction we want to go. --- .../rustc_llvm/llvm-wrapper/RustWrapper.cpp | 39 ++----------------- 1 file changed, 3 insertions(+), 36 deletions(-) diff --git a/compiler/rustc_llvm/llvm-wrapper/RustWrapper.cpp b/compiler/rustc_llvm/llvm-wrapper/RustWrapper.cpp index b3143f14d29cc..3a7af73c87de3 100644 --- a/compiler/rustc_llvm/llvm-wrapper/RustWrapper.cpp +++ b/compiler/rustc_llvm/llvm-wrapper/RustWrapper.cpp @@ -207,18 +207,7 @@ template static inline void AddAttribute(T *t, unsigned Index, Attri #if LLVM_VERSION_LT(14, 0) t->addAttribute(Index, Attr); #else - // TODO(durin42): we should probably surface the explicit functions to Rust - // instead of this switch statement? - switch (Index) { - case AttributeList::ReturnIndex: - t->addRetAttr(Attr); - break; - case AttributeList::FunctionIndex: - t->addFnAttr(Attr); - break; - default: - t->addParamAttr(Index-AttributeList::FirstArgIndex, Attr); - } + t->addAttributeAtIndex(Index, Attr); #endif } @@ -241,18 +230,7 @@ static inline void AddCallAttributes(CallBase *Call, unsigned Index, const AttrB #if LLVM_VERSION_LT(14, 0) Attrs = Attrs.addAttributes(Call->getContext(), Index, B); #else - // TODO(durin42): we should probably surface the explicit functions to Rust - // instead of this switch statement? - switch (Index) { - case AttributeList::ReturnIndex: - Attrs = Attrs.addRetAttributes(Call->getContext(), B); - break; - case AttributeList::FunctionIndex: - Attrs = Attrs.addFnAttributes(Call->getContext(), B); - break; - default: - Attrs = Attrs.addParamAttributes(Call->getContext(), Index-AttributeList::FirstArgIndex, B); - } + Attrs = Attrs.addAttributesAtIndex(Call->getContext(), Index, B); #endif Call->setAttributes(Attrs); } @@ -370,18 +348,7 @@ extern "C" void LLVMRustRemoveFunctionAttributes(LLVMValueRef Fn, #if LLVM_VERSION_LT(14, 0) PALNew = PAL.removeAttributes(F->getContext(), Index, B); #else - // TODO(durin42): we should probably surface the explicit functions to Rust - // instead of this switch statement? - switch (Index) { - case AttributeList::ReturnIndex: - PALNew = PAL.removeRetAttributes(F->getContext(), B); - break; - case AttributeList::FunctionIndex: - PALNew = PAL.removeFnAttributes(F->getContext(), B); - break; - default: - PALNew = PAL.removeParamAttributes(F->getContext(), Index-AttributeList::FirstArgIndex, B); - } + PALNew = PAL.removeAttributesAtIndex(F->getContext(), Index, B); #endif F->setAttributes(PALNew); } From 4d045406d1ee4c011e476fbeae81976c8012e886 Mon Sep 17 00:00:00 2001 From: Augie Fackler Date: Wed, 8 Sep 2021 10:47:41 -0400 Subject: [PATCH 3/3] RustWrapper: remove some uses of AttrBuilder Turns out we can also use Attribute::get*() methods here, and avoid the AttrBuilder and an extra helper method here. --- .../rustc_llvm/llvm-wrapper/RustWrapper.cpp | 25 +++++-------------- 1 file changed, 6 insertions(+), 19 deletions(-) diff --git a/compiler/rustc_llvm/llvm-wrapper/RustWrapper.cpp b/compiler/rustc_llvm/llvm-wrapper/RustWrapper.cpp index 3a7af73c87de3..9850f395a0f65 100644 --- a/compiler/rustc_llvm/llvm-wrapper/RustWrapper.cpp +++ b/compiler/rustc_llvm/llvm-wrapper/RustWrapper.cpp @@ -225,41 +225,28 @@ extern "C" void LLVMRustAddCallSiteAttrString(LLVMValueRef Instr, unsigned Index AddAttribute(Call, Index, Attr); } -static inline void AddCallAttributes(CallBase *Call, unsigned Index, const AttrBuilder& B) { - AttributeList Attrs = Call->getAttributes(); -#if LLVM_VERSION_LT(14, 0) - Attrs = Attrs.addAttributes(Call->getContext(), Index, B); -#else - Attrs = Attrs.addAttributesAtIndex(Call->getContext(), Index, B); -#endif - Call->setAttributes(Attrs); -} - extern "C" void LLVMRustAddAlignmentCallSiteAttr(LLVMValueRef Instr, unsigned Index, uint32_t Bytes) { CallBase *Call = unwrap(Instr); - AttrBuilder B; - B.addAlignmentAttr(Bytes); - AddCallAttributes(Call, Index, B); + Attribute Attr = Attribute::getWithAlignment(Call->getContext(), Align(Bytes)); + AddAttribute(Call, Index, Attr); } extern "C" void LLVMRustAddDereferenceableCallSiteAttr(LLVMValueRef Instr, unsigned Index, uint64_t Bytes) { CallBase *Call = unwrap(Instr); - AttrBuilder B; - B.addDereferenceableAttr(Bytes); - AddCallAttributes(Call, Index, B); + Attribute Attr = Attribute::getWithDereferenceableBytes(Call->getContext(), Bytes); + AddAttribute(Call, Index, Attr); } extern "C" void LLVMRustAddDereferenceableOrNullCallSiteAttr(LLVMValueRef Instr, unsigned Index, uint64_t Bytes) { CallBase *Call = unwrap(Instr); - AttrBuilder B; - B.addDereferenceableOrNullAttr(Bytes); - AddCallAttributes(Call, Index, B); + Attribute Attr = Attribute::getWithDereferenceableOrNullBytes(Call->getContext(), Bytes); + AddAttribute(Call, Index, Attr); } extern "C" void LLVMRustAddByValCallSiteAttr(LLVMValueRef Instr, unsigned Index,