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.

Sign up for GitHub

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

Jump to bottom

cleanup: remove pointee types #105545

Merged
merged 5 commits into from
Aug 1, 2023
Merged

cleanup: remove pointee types #105545

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift click to select a range
b654077
cg_llvm: remove pointee types and pointercast/bitcast-of-ptr
erikdesjardins Dec 6, 2022
04303cf
cg_ssa: remove pointee types and pointercast/bitcast-of-ptr
erikdesjardins Jul 29, 2023
def44c5
cg_llvm: inline check_store
erikdesjardins Jul 29, 2023
cf7788d
cg_llvm: clean up match
erikdesjardins Jul 29, 2023
5580012
cg_llvm: simplify llvm.masked.gather/scatter naming with opaque pointers
erikdesjardins Jul 29, 2023
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
Next Next commit
cg_llvm: remove pointee types and pointercast/bitcast-of-ptr
  • Loading branch information
@erikdesjardins
erikdesjardins committed Jul 29, 2023
commit b6540777fed3ca92a7c306be6c6ab4b074a033b2
17 changes: 4 additions & 13 deletions compiler/rustc_codegen_llvm/src/abi.rs
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
35 changes: 9 additions & 26 deletions compiler/rustc_codegen_llvm/src/builder.rs
View file
Open in desktop
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);
let ptr = self.check_store(ptr);
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);
let ptr = self.check_store(ptr);
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,23 1382,12 @@ 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 {
fn check_store(&mut self, ptr: &'ll Value) -> &'ll Value {
bjorn3 marked this conversation as resolved.
Show resolved Hide resolved
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)
}
ptr
}

fn check_call<'b>(
Expand Down Expand Up @@ -1468,7 1452,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
View file
Open in desktop
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
12 changes: 3 additions & 9 deletions compiler/rustc_codegen_llvm/src/common.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 4,6 @@ use crate::consts::{self, const_alloc_to_llvm};
pub use crate::context::CodegenCx;
use crate::llvm::{self, BasicBlock, Bool, ConstantInt, False, OperandBundleDef, True};
use crate::type_::Type;
use crate::type_of::LayoutLlvmExt;
use crate::value::Value;

use rustc_ast::Mutability;
Expand All @@ -13,7 12,6 @@ use rustc_data_structures::stable_hasher::{Hash128, HashStable, StableHasher};
use rustc_hir::def_id::DefId;
use rustc_middle::bug;
use rustc_middle::mir::interpret::{ConstAllocation, GlobalAlloc, Scalar};
use rustc_middle::ty::layout::LayoutOf;
use rustc_middle::ty::TyCtxt;
use rustc_session::cstore::{DllCallingConvention, DllImport, PeImportNameType};
use rustc_target::abi::{self, AddressSpace, HasDataLayout, Pointer};
Expand Down Expand Up @@ -211,11 209,7 @@ impl<'ll, 'tcx> ConstMethods<'tcx> for CodegenCx<'ll, 'tcx> {
})
.1;
let len = s.len();
let cs = consts::ptrcast(
str_global,
self.type_ptr_to(self.layout_of(self.tcx.types.str_).llvm_type(self)),
);
(cs, self.const_usize(len as u64))
(str_global, self.const_usize(len as u64))
}

fn const_struct(&self, elts: &[&'ll Value], packed: bool) -> &'ll Value {
Expand Down Expand Up @@ -292,7 286,7 @@ impl<'ll, 'tcx> ConstMethods<'tcx> for CodegenCx<'ll, 'tcx> {
let llval = unsafe {
llvm::LLVMConstInBoundsGEP2(
self.type_i8(),
self.const_bitcast(base_addr, self.type_i8p_ext(base_addr_space)),
self.const_bitcast(base_addr, self.type_ptr_ext(base_addr_space)),
&self.const_usize(offset.bytes()),
1,
)
Expand Down Expand Up @@ -322,7 316,7 @@ impl<'ll, 'tcx> ConstMethods<'tcx> for CodegenCx<'ll, 'tcx> {
unsafe {
llvm::LLVMConstInBoundsGEP2(
self.type_i8(),
self.const_bitcast(base_addr, self.type_i8p()),
base_addr,
&self.const_usize(offset.bytes()),
1,
)
Expand Down
16 changes: 7 additions & 9 deletions compiler/rustc_codegen_llvm/src/consts.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 103,7 @@ pub fn const_alloc_to_llvm<'ll>(cx: &CodegenCx<'ll, '_>, alloc: ConstAllocation<
value: Primitive::Pointer(address_space),
valid_range: WrappingRange::full(dl.pointer_size),
},
cx.type_i8p_ext(address_space),
cx.type_ptr_ext(address_space),
));
next_offset = offset pointer_size;
}
Expand Down Expand Up @@ -179,7 179,7 @@ fn check_and_apply_linkage<'ll, 'tcx>(
})
});
llvm::LLVMRustSetLinkage(g2, llvm::Linkage::InternalLinkage);
llvm::LLVMSetInitializer(g2, cx.const_ptrcast(g1, llty));
llvm::LLVMSetInitializer(g2, g1);
g2
}
} else if cx.tcx.sess.target.arch == "x86" &&
Expand Down Expand Up @@ -251,7 251,7 @@ impl<'ll> CodegenCx<'ll, '_> {
let g = if def_id.is_local() && !self.tcx.is_foreign_item(def_id) {
let llty = self.layout_of(ty).llvm_type(self);
if let Some(g) = self.get_declared_value(sym) {
if self.val_ty(g) != self.type_ptr_to(llty) {
if self.val_ty(g) != self.type_ptr() {
span_bug!(self.tcx.def_span(def_id), "Conflicting types for static");
}
}
Expand Down Expand Up @@ -552,16 552,14 @@ impl<'ll> StaticMethods for CodegenCx<'ll, '_> {
}
}

/// Add a global value to a list to be stored in the `llvm.used` variable, an array of i8*.
/// Add a global value to a list to be stored in the `llvm.used` variable, an array of ptr.
fn add_used_global(&self, global: &'ll Value) {
let cast = unsafe { llvm::LLVMConstPointerCast(global, self.type_i8p()) };
self.used_statics.borrow_mut().push(cast);
self.used_statics.borrow_mut().push(global);
}

/// Add a global value to a list to be stored in the `llvm.compiler.used` variable,
/// an array of i8*.
/// an array of ptr.
fn add_compiler_used_global(&self, global: &'ll Value) {
let cast = unsafe { llvm::LLVMConstPointerCast(global, self.type_i8p()) };
self.compiler_used_statics.borrow_mut().push(cast);
self.compiler_used_statics.borrow_mut().push(global);
}
}
Loading