Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cleanup: remove pointee types #105545

Merged
merged 5 commits into from
Aug 1, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion compiler/rustc_codegen_gcc/src/builder.rs
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
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
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
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
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
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
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
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
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 @@ -1468,7 +1444,6 @@ impl<'a, 'll, 'tcx> Builder<'a, 'll, 'tcx> {
return;
}

let ptr = self.pointercast(ptr, self.cx.type_i8p());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can pointercast be implemented as nop after these changes?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It can still be used to convert between different pointer address spaces, e.g. when casting data pointers to function pointers on harvard architecture targets: https://godbolt.org/z/c4nEb8sTW. (Note that we don't use pointercast for this today; that IR is actually invalid as a result. I'll open a PR to fix it.)

On other targets it'll always do nothing. The LLVM side has a check to avoid creating a new instruction if the pointer is already the correct type.

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
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
}
erikdesjardins marked this conversation as resolved.
Show resolved Hide resolved
llfn
} else {
let instance_def_id = instance.def_id();
let llfn = if tcx.sess.target.arch == "x86" &&
Expand Down
Loading