Skip to content

Commit

Permalink
Auto merge of #105545 - erikdesjardins:ptrclean, r=bjorn3
Browse files Browse the repository at this point in the history 
cleanup: remove pointee types

This can't be merged until the oldest LLVM version we support uses opaque pointers, which will be the case after #114148. (Also note `-Cllvm-args="-opaque-pointers=0"` can technically be used in LLVM 15, though I don't think we should support that configuration.)

I initially hoped this would provide some minor perf win, but in #105412 (comment) it had very little impact, so this is only valuable as a cleanup.

As a followup, this will enable #96242 to be resolved.

r? `@ghost`

`@rustbot` label S-blocked
  • Loading branch information
@bors
bors committed Aug 1, 2023
2 parents fe90d7d 5580012 commit abd3637
Show file tree
Hide file tree
Showing 32 changed files with 227 additions and 584 deletions.
1 change: 0 additions & 1 deletion compiler/rustc_codegen_gcc/src/builder.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 27,6 @@ use rustc_codegen_ssa::traits::{
BaseTypeMethods,
BuilderMethods,
ConstMethods,
DerivedTypeMethods,
LayoutTypeMethods,
HasCodegen,
OverflowOp,
Expand Down
8 changes: 4 additions & 4 deletions compiler/rustc_codegen_gcc/src/common.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 16,10 @@ use crate::context::CodegenCx;
use crate::type_of::LayoutGccExt;

impl<'gcc, 'tcx> CodegenCx<'gcc, 'tcx> {
pub fn const_ptrcast(&self, val: RValue<'gcc>, ty: Type<'gcc>) -> RValue<'gcc> {
self.context.new_cast(None, val, ty)
}

pub fn const_bytes(&self, bytes: &[u8]) -> RValue<'gcc> {
bytes_in_context(self, bytes)
}
Expand Down Expand Up @@ -242,10 246,6 @@ impl<'gcc, 'tcx> ConstMethods<'tcx> for CodegenCx<'gcc, 'tcx> {
const_alloc_to_gcc(self, alloc)
}

fn const_ptrcast(&self, val: RValue<'gcc>, ty: Type<'gcc>) -> RValue<'gcc> {
self.context.new_cast(None, val, ty)
}

fn const_bitcast(&self, value: RValue<'gcc>, typ: Type<'gcc>) -> RValue<'gcc> {
if value.get_type() == self.bool_type.make_pointer() {
if let Some(pointee) = typ.get_pointee() {
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_codegen_gcc/src/intrinsic/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 12,7 @@ use rustc_codegen_ssa::mir::operand::{OperandRef, OperandValue};
use rustc_codegen_ssa::mir::place::PlaceRef;
use rustc_codegen_ssa::traits::{ArgAbiMethods, BaseTypeMethods, BuilderMethods, ConstMethods, IntrinsicCallMethods};
#[cfg(feature="master")]
use rustc_codegen_ssa::traits::{DerivedTypeMethods, MiscMethods};
use rustc_codegen_ssa::traits::MiscMethods;
use rustc_codegen_ssa::errors::InvalidMonomorphization;
use rustc_middle::bug;
use rustc_middle::ty::{self, Instance, Ty};
Expand Down
26 changes: 21 additions & 5 deletions compiler/rustc_codegen_gcc/src/type_.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 54,23 @@ impl<'gcc, 'tcx> CodegenCx<'gcc, 'tcx> {
self.u128_type
}

pub fn type_ptr_to(&self, ty: Type<'gcc>) -> Type<'gcc> {
ty.make_pointer()
}

pub fn type_ptr_to_ext(&self, ty: Type<'gcc>, _address_space: AddressSpace) -> Type<'gcc> {
// TODO(antoyo): use address_space, perhaps with TYPE_ADDR_SPACE?
ty.make_pointer()
}

pub fn type_i8p(&self) -> Type<'gcc> {
self.type_ptr_to(self.type_i8())
}

pub fn type_i8p_ext(&self, address_space: AddressSpace) -> Type<'gcc> {
self.type_ptr_to_ext(self.type_i8(), address_space)
}

pub fn type_pointee_for_align(&self, align: Align) -> Type<'gcc> {
// FIXME(eddyb) We could find a better approximation if ity.align < align.
let ity = Integer::approximate_align(self, align);
Expand Down Expand Up @@ -149,13 166,12 @@ impl<'gcc, 'tcx> BaseTypeMethods<'tcx> for CodegenCx<'gcc, 'tcx> {
}
}

fn type_ptr_to(&self, ty: Type<'gcc>) -> Type<'gcc> {
ty.make_pointer()
fn type_ptr(&self) -> Type<'gcc> {
self.type_ptr_to(self.type_void())
}

fn type_ptr_to_ext(&self, ty: Type<'gcc>, _address_space: AddressSpace) -> Type<'gcc> {
// TODO(antoyo): use address_space, perhaps with TYPE_ADDR_SPACE?
ty.make_pointer()
fn type_ptr_ext(&self, address_space: AddressSpace) -> Type<'gcc> {
self.type_ptr_to_ext(self.type_void(), address_space)
}

fn element_type(&self, ty: Type<'gcc>) -> Type<'gcc> {
Expand Down
17 changes: 4 additions & 13 deletions compiler/rustc_codegen_llvm/src/abi.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -216,9 216,7 @@ impl<'ll, 'tcx> ArgAbiExt<'ll, 'tcx> for ArgAbi<'tcx, Ty<'tcx>> {
// uses it for i16 -> {i8, i8}, but not for i24 -> {i8, i8, i8}.
let can_store_through_cast_ptr = false;
if can_store_through_cast_ptr {
let cast_ptr_llty = bx.type_ptr_to(cast.llvm_type(bx));
let cast_dst = bx.pointercast(dst.llval, cast_ptr_llty);
bx.store(val, cast_dst, self.layout.align.abi);
bx.store(val, dst.llval, self.layout.align.abi);
} else {
// The actual return type is a struct, but the ABI
// adaptation code has cast it into some scalar type. The
Expand Down Expand Up @@ -336,7 334,7 @@ impl<'ll, 'tcx> FnAbiLlvmExt<'ll, 'tcx> for FnAbi<'tcx, Ty<'tcx>> {
PassMode::Direct(_) | PassMode::Pair(..) => self.ret.layout.immediate_llvm_type(cx),
PassMode::Cast(cast, _) => cast.llvm_type(cx),
PassMode::Indirect { .. } => {
llargument_tys.push(cx.type_ptr_to(self.ret.memory_ty(cx)));
llargument_tys.push(cx.type_ptr());
cx.type_void()
}
};
Expand Down Expand Up @@ -364,9 362,7 @@ impl<'ll, 'tcx> FnAbiLlvmExt<'ll, 'tcx> for FnAbi<'tcx, Ty<'tcx>> {
}
cast.llvm_type(cx)
}
PassMode::Indirect { attrs: _, extra_attrs: None, on_stack: _ } => {
cx.type_ptr_to(arg.memory_ty(cx))
}
PassMode::Indirect { attrs: _, extra_attrs: None, on_stack: _ } => cx.type_ptr(),
};
llargument_tys.push(llarg_ty);
}
Expand All @@ -379,12 375,7 @@ impl<'ll, 'tcx> FnAbiLlvmExt<'ll, 'tcx> for FnAbi<'tcx, Ty<'tcx>> {
}

fn ptr_to_llvm_type(&self, cx: &CodegenCx<'ll, 'tcx>) -> &'ll Type {
unsafe {
llvm::LLVMPointerType(
self.llvm_type(cx),
cx.data_layout().instruction_address_space.0 as c_uint,
)
}
cx.type_ptr_ext(cx.data_layout().instruction_address_space)
}

fn llvm_cconv(&self) -> llvm::CallConv {
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_codegen_llvm/src/allocator.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 28,7 @@ pub(crate) unsafe fn codegen(
tws => bug!("Unsupported target word size for int: {}", tws),
};
let i8 = llvm::LLVMInt8TypeInContext(llcx);
let i8p = llvm::LLVMPointerType(i8, 0);
let i8p = llvm::LLVMPointerTypeInContext(llcx, 0);
let void = llvm::LLVMVoidTypeInContext(llcx);

if kind == AllocatorKind::Default {
Expand Down
7 changes: 3 additions & 4 deletions compiler/rustc_codegen_llvm/src/back/write.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 4,6 @@ use crate::back::profiling::{
};
use crate::base;
use crate::common;
use crate::consts;
use crate::errors::{
CopyBitcode, FromLlvmDiag, FromLlvmOptimizationDiag, LlvmError, WithLlvmError, WriteBytecode,
};
Expand Down Expand Up @@ -992,7 991,7 @@ fn create_msvc_imps(
let prefix = if cgcx.target_arch == "x86" { "\x01__imp__" } else { "\x01__imp_" };

unsafe {
let i8p_ty = Type::i8p_llcx(llcx);
let ptr_ty = Type::ptr_llcx(llcx);
let globals = base::iter_globals(llmod)
.filter(|&val| {
llvm::LLVMRustGetLinkage(val) == llvm::Linkage::ExternalLinkage
Expand All @@ -1012,8 1011,8 @@ fn create_msvc_imps(
.collect::<Vec<_>>();

for (imp_name, val) in globals {
let imp = llvm::LLVMAddGlobal(llmod, i8p_ty, imp_name.as_ptr().cast());
llvm::LLVMSetInitializer(imp, consts::ptrcast(val, i8p_ty));
let imp = llvm::LLVMAddGlobal(llmod, ptr_ty, imp_name.as_ptr().cast());
llvm::LLVMSetInitializer(imp, val);
llvm::LLVMRustSetLinkage(imp, llvm::Linkage::ExternalLinkage);
}
}
Expand Down
3 changes: 1 addition & 2 deletions compiler/rustc_codegen_llvm/src/base.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -123,8 123,7 @@ pub fn compile_codegen_unit(tcx: TyCtxt<'_>, cgu_name: Symbol) -> (ModuleCodegen
// happen after the llvm.used variables are created.
for &(old_g, new_g) in cx.statics_to_rauw().borrow().iter() {
unsafe {
let bitcast = llvm::LLVMConstPointerCast(new_g, cx.val_ty(old_g));
llvm::LLVMReplaceAllUsesWith(old_g, bitcast);
llvm::LLVMReplaceAllUsesWith(old_g, new_g);
llvm::LLVMDeleteGlobal(old_g);
}
}
Expand Down
39 changes: 7 additions & 32 deletions compiler/rustc_codegen_llvm/src/builder.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -652,7 652,7 @@ impl<'a, 'll, 'tcx> BuilderMethods<'a, 'tcx> for Builder<'a, 'll, 'tcx> {
flags: MemFlags,
) -> &'ll Value {
debug!("Store {:?} -> {:?} ({:?})", val, ptr, flags);
let ptr = self.check_store(val, ptr);
assert_eq!(self.cx.type_kind(self.cx.val_ty(ptr)), TypeKind::Pointer);
unsafe {
let store = llvm::LLVMBuildStore(self.llbuilder, val, ptr);
let align =
Expand Down Expand Up @@ -682,7 682,7 @@ impl<'a, 'll, 'tcx> BuilderMethods<'a, 'tcx> for Builder<'a, 'll, 'tcx> {
size: Size,
) {
debug!("Store {:?} -> {:?}", val, ptr);
let ptr = self.check_store(val, ptr);
assert_eq!(self.cx.type_kind(self.cx.val_ty(ptr)), TypeKind::Pointer);
unsafe {
let store = llvm::LLVMRustBuildAtomicStore(
self.llbuilder,
Expand Down Expand Up @@ -873,8 873,6 @@ impl<'a, 'll, 'tcx> BuilderMethods<'a, 'tcx> for Builder<'a, 'll, 'tcx> {
assert!(!flags.contains(MemFlags::NONTEMPORAL), "non-temporal memcpy not supported");
let size = self.intcast(size, self.type_isize(), false);
let is_volatile = flags.contains(MemFlags::VOLATILE);
let dst = self.pointercast(dst, self.type_i8p());
let src = self.pointercast(src, self.type_i8p());
unsafe {
llvm::LLVMRustBuildMemCpy(
self.llbuilder,
Expand All @@ -900,8 898,6 @@ impl<'a, 'll, 'tcx> BuilderMethods<'a, 'tcx> for Builder<'a, 'll, 'tcx> {
assert!(!flags.contains(MemFlags::NONTEMPORAL), "non-temporal memmove not supported");
let size = self.intcast(size, self.type_isize(), false);
let is_volatile = flags.contains(MemFlags::VOLATILE);
let dst = self.pointercast(dst, self.type_i8p());
let src = self.pointercast(src, self.type_i8p());
unsafe {
llvm::LLVMRustBuildMemMove(
self.llbuilder,
Expand All @@ -924,7 920,6 @@ impl<'a, 'll, 'tcx> BuilderMethods<'a, 'tcx> for Builder<'a, 'll, 'tcx> {
flags: MemFlags,
) {
let is_volatile = flags.contains(MemFlags::VOLATILE);
let ptr = self.pointercast(ptr, self.type_i8p());
unsafe {
llvm::LLVMRustBuildMemSet(
self.llbuilder,
Expand Down Expand Up @@ -981,7 976,7 @@ impl<'a, 'll, 'tcx> BuilderMethods<'a, 'tcx> for Builder<'a, 'll, 'tcx> {
}

fn cleanup_landing_pad(&mut self, pers_fn: &'ll Value) -> (&'ll Value, &'ll Value) {
let ty = self.type_struct(&[self.type_i8p(), self.type_i32()], false);
let ty = self.type_struct(&[self.type_ptr(), self.type_i32()], false);
let landing_pad = self.landing_pad(ty, pers_fn, 0);
unsafe {
llvm::LLVMSetCleanup(landing_pad, llvm::True);
Expand All @@ -990,14 985,14 @@ impl<'a, 'll, 'tcx> BuilderMethods<'a, 'tcx> for Builder<'a, 'll, 'tcx> {
}

fn filter_landing_pad(&mut self, pers_fn: &'ll Value) -> (&'ll Value, &'ll Value) {
let ty = self.type_struct(&[self.type_i8p(), self.type_i32()], false);
let ty = self.type_struct(&[self.type_ptr(), self.type_i32()], false);
let landing_pad = self.landing_pad(ty, pers_fn, 1);
self.add_clause(landing_pad, self.const_array(self.type_i8p(), &[]));
self.add_clause(landing_pad, self.const_array(self.type_ptr(), &[]));
(self.extract_value(landing_pad, 0), self.extract_value(landing_pad, 1))
}

fn resume(&mut self, exn0: &'ll Value, exn1: &'ll Value) {
let ty = self.type_struct(&[self.type_i8p(), self.type_i32()], false);
let ty = self.type_struct(&[self.type_ptr(), self.type_i32()], false);
let mut exn = self.const_poison(ty);
exn = self.insert_value(exn, exn0, 0);
exn = self.insert_value(exn, exn1, 1);
Expand Down Expand Up @@ -1161,7 1156,7 @@ impl<'a, 'll, 'tcx> BuilderMethods<'a, 'tcx> for Builder<'a, 'll, 'tcx> {

let llfn = unsafe { llvm::LLVMRustGetInstrProfIncrementIntrinsic(self.cx().llmod) };
let llty = self.cx.type_func(
&[self.cx.type_i8p(), self.cx.type_i64(), self.cx.type_i32(), self.cx.type_i32()],
&[self.cx.type_ptr(), self.cx.type_i64(), self.cx.type_i32(), self.cx.type_i32()],
self.cx.type_void(),
);
let args = &[fn_name, hash, num_counters, index];
Expand Down Expand Up @@ -1387,25 1382,6 @@ impl<'a, 'll, 'tcx> Builder<'a, 'll, 'tcx> {
ret.expect("LLVM does not have support for catchret")
}

fn check_store(&mut self, val: &'ll Value, ptr: &'ll Value) -> &'ll Value {
let dest_ptr_ty = self.cx.val_ty(ptr);
let stored_ty = self.cx.val_ty(val);
let stored_ptr_ty = self.cx.type_ptr_to(stored_ty);

assert_eq!(self.cx.type_kind(dest_ptr_ty), TypeKind::Pointer);

if dest_ptr_ty == stored_ptr_ty {
ptr
} else {
debug!(
"type mismatch in store. \
Expected {:?}, got {:?}; inserting bitcast",
dest_ptr_ty, stored_ptr_ty
);
self.bitcast(ptr, stored_ptr_ty)
}
}

fn check_call<'b>(
&mut self,
typ: &str,
Expand Down Expand Up @@ -1466,7 1442,6 @@ impl<'a, 'll, 'tcx> Builder<'a, 'll, 'tcx> {
return;
}

let ptr = self.pointercast(ptr, self.cx.type_i8p());
self.call_intrinsic(intrinsic, &[self.cx.const_u64(size), ptr]);
}

Expand Down
36 changes: 1 addition & 35 deletions compiler/rustc_codegen_llvm/src/callee.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 4,11 @@
//! and methods are represented as just a fn ptr and not a full
//! closure.

use crate::abi::FnAbiLlvmExt;
use crate::attributes;
use crate::common;
use crate::context::CodegenCx;
use crate::llvm;
use crate::value::Value;
use rustc_codegen_ssa::traits::*;

use rustc_middle::ty::layout::{FnAbiOf, HasTyCtxt};
use rustc_middle::ty::{self, Instance, TypeVisitableExt};
Expand Down Expand Up @@ -45,39 43,7 @@ pub fn get_fn<'ll, 'tcx>(cx: &CodegenCx<'ll, 'tcx>, instance: Instance<'tcx>) ->
let fn_abi = cx.fn_abi_of_instance(instance, ty::List::empty());

let llfn = if let Some(llfn) = cx.get_declared_value(sym) {
// Create a fn pointer with the new signature.
let llptrty = fn_abi.ptr_to_llvm_type(cx);

// This is subtle and surprising, but sometimes we have to bitcast
// the resulting fn pointer. The reason has to do with external
// functions. If you have two crates that both bind the same C
// library, they may not use precisely the same types: for
// example, they will probably each declare their own structs,
// which are distinct types from LLVM's point of view (nominal
// types).
//
// Now, if those two crates are linked into an application, and
// they contain inlined code, you can wind up with a situation
// where both of those functions wind up being loaded into this
// application simultaneously. In that case, the same function
// (from LLVM's point of view) requires two types. But of course
// LLVM won't allow one function to have two types.
//
// What we currently do, therefore, is declare the function with
// one of the two types (whichever happens to come first) and then
// bitcast as needed when the function is referenced to make sure
// it has the type we expect.
//
// This can occur on either a crate-local or crate-external
// reference. It also occurs when testing libcore and in some
// other weird situations. Annoying.
if cx.val_ty(llfn) != llptrty {
debug!("get_fn: casting {:?} to {:?}", llfn, llptrty);
cx.const_ptrcast(llfn, llptrty)
} else {
debug!("get_fn: not casting pointer!");
llfn
}
llfn
} else {
let instance_def_id = instance.def_id();
let llfn = if tcx.sess.target.arch == "x86" &&
Expand Down
Loading

0 comments on commit abd3637

Please sign in to comment.