Skip to content

Refactor the way cg_ssa handles indirect returns(returns via sret). #144976

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

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Refactor the way cg_ssa handles indirect returns(returns via sret). #144976

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
400c93c
Refactored the handling of the indirect return pointer in cg_ssa to a…
FractalFir Aug 5, 2025
1c33aae
Ported cg_gcc to the new cg_ssa version
FractalFir Aug 5, 2025
93adb78
Apply suggestions from code review
FractalFir Sep 19, 2025
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
11 changes: 10 additions & 1 deletion compiler/rustc_codegen_gcc/src/asm.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -563,7 +563,16 @@ impl<'a, 'gcc, 'tcx> AsmBuilderMethods<'tcx> for Builder<'a, 'gcc, 'tcx> {
let builtin_unreachable = self.context.get_builtin_function("__builtin_unreachable");
let builtin_unreachable: RValue<'gcc> =
unsafe { std::mem::transmute(builtin_unreachable) };
self.call(self.type_void(), None, None, builtin_unreachable, &[], None, None);
self.call(
self.type_void(),
None,
None,
builtin_unreachable,
None, /* Assembly can't return indirectly. */
&[],
None,
None,
);
}

// Write results to outputs.
Expand Down
23 changes: 19 additions & 4 deletions compiler/rustc_codegen_gcc/src/builder.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -600,6 +600,7 @@ impl<'a, 'gcc, 'tcx> BuilderMethods<'a, 'tcx> for Builder<'a, 'gcc, 'tcx> {
fn_attrs: Option<&CodegenFnAttrs>,
_fn_abi: Option<&FnAbi<'tcx, Ty<'tcx>>>,
func: RValue<'gcc>,
indirect_return_pointer: Option<RValue<'gcc>>,
args: &[RValue<'gcc>],
then: Block<'gcc>,
catch: Block<'gcc>,
Expand All @@ -610,7 +611,8 @@ impl<'a, 'gcc, 'tcx> BuilderMethods<'a, 'tcx> for Builder<'a, 'gcc, 'tcx> {

let current_block = self.block;
self.block = try_block;
let call = self.call(typ, fn_attrs, None, func, args, None, instance); // TODO(antoyo): use funclet here?
let call =
self.call(typ, fn_attrs, None, func, indirect_return_pointer, args, None, instance); // TODO(antoyo): use funclet here?
Comment on lines -613 to +615
Copy link
Member

Choose a reason for hiding this comment

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

please break the comment onto the preceding line.

self.block = current_block;

let return_value =
Expand Down Expand Up @@ -638,13 +640,15 @@ impl<'a, 'gcc, 'tcx> BuilderMethods<'a, 'tcx> for Builder<'a, 'gcc, 'tcx> {
fn_attrs: Option<&CodegenFnAttrs>,
fn_abi: Option<&FnAbi<'tcx, Ty<'tcx>>>,
func: RValue<'gcc>,
indirect_return_pointer: Option<RValue<'gcc>>,
args: &[RValue<'gcc>],
then: Block<'gcc>,
catch: Block<'gcc>,
_funclet: Option<&Funclet>,
instance: Option<Instance<'tcx>>,
) -> RValue<'gcc> {
let call_site = self.call(typ, fn_attrs, None, func, args, None, instance);
let call_site =
self.call(typ, fn_attrs, None, func, indirect_return_pointer, args, None, instance);
let condition = self.context.new_rvalue_from_int(self.bool_type, 1);
self.llbb().end_with_conditional(self.location, condition, then, catch);
if let Some(_fn_abi) = fn_abi {
Expand Down Expand Up @@ -1739,17 +1743,28 @@ impl<'a, 'gcc, 'tcx> BuilderMethods<'a, 'tcx> for Builder<'a, 'gcc, 'tcx> {
_fn_attrs: Option<&CodegenFnAttrs>,
fn_abi: Option<&FnAbi<'tcx, Ty<'tcx>>>,
func: RValue<'gcc>,
indirect_return_pointer: Option<RValue<'gcc>>,
args: &[RValue<'gcc>],
funclet: Option<&Funclet>,
_instance: Option<Instance<'tcx>>,
) -> RValue<'gcc> {
// FIXME: change this in the `rustc_codegen_gcc` repo after the sync, to use the `libgccjit` indirect return suppport.
let args = match indirect_return_pointer {
None => args.to_vec(),
Some(sret_ptr) => {
let mut args = args.to_vec();
// Preappend the indirect return pointer
args.insert(0, sret_ptr);
args
}
};
// FIXME(antoyo): remove when having a proper API.
let gcc_func = unsafe { std::mem::transmute::<RValue<'gcc>, Function<'gcc>>(func) };
let call = if self.functions.borrow().values().any(|value| *value == gcc_func) {
self.function_call(func, args, funclet)
self.function_call(func, &args, funclet)
} else {
// If it's a not function that was defined, it's a function pointer.
self.function_ptr_call(typ, func, args, funclet)
self.function_ptr_call(typ, func, &args, funclet)
};
if let Some(_fn_abi) = fn_abi {
// TODO(bjorn3): Apply function attributes
Expand Down
57 changes: 52 additions & 5 deletions compiler/rustc_codegen_gcc/src/intrinsic/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -644,7 +644,16 @@ impl<'a, 'gcc, 'tcx> IntrinsicCallBuilderMethods<'tcx> for Builder<'a, 'gcc, 'tc
fn abort(&mut self) {
let func = self.context.get_builtin_function("abort");
let func: RValue<'gcc> = unsafe { std::mem::transmute(func) };
self.call(self.type_void(), None, None, func, &[], None, None);
self.call(
self.type_void(),
None,
None,
func,
None, /* abort does not return, so it can't return indirectly. */
&[],
None,
None,
);
}

fn assume(&mut self, value: Self::Value) {
Expand Down Expand Up @@ -1340,7 +1349,16 @@ fn try_intrinsic<'a, 'b, 'gcc, 'tcx>(
dest: PlaceRef<'tcx, RValue<'gcc>>,
) {
if bx.sess().panic_strategy() == PanicStrategy::Abort {
bx.call(bx.type_void(), None, None, try_func, &[data], None, None);
bx.call(
bx.type_void(),
None,
None,
try_func,
None, /* This intrinsic does not return indirectly.*/
&[data],
None,
None,
);
// Return 0 unconditionally from the intrinsic call;
// we can never unwind.
OperandValue::Immediate(bx.const_i32(0)).store(bx, dest);
Expand Down Expand Up @@ -1413,21 +1431,50 @@ fn codegen_gnu_try<'gcc, 'tcx>(
let zero = bx.cx.context.new_rvalue_zero(bx.int_type);
let ptr = bx.cx.context.new_call(None, eh_pointer_builtin, &[zero]);
let catch_ty = bx.type_func(&[bx.type_i8p(), bx.type_i8p()], bx.type_void());
bx.call(catch_ty, None, None, catch_func, &[data, ptr], None, None);
bx.call(
catch_ty,
None,
None,
catch_func,
None, /* this function can't return indirectly */
Copy link
Member

Choose a reason for hiding this comment

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

if there is going to be a comment, then please answer "why can't it?" instead of a statement that it doesn't return indirectly.

&[data, ptr],
None,
None,
);
bx.ret(bx.const_i32(1));

// NOTE: the blocks must be filled before adding the try/catch, otherwise gcc will not
// generate a try/catch.
// FIXME(antoyo): add a check in the libgccjit API to prevent this.
bx.switch_to_block(current_block);
bx.invoke(try_func_ty, None, None, try_func, &[data], then, catch, None, None);
bx.invoke(
try_func_ty,
None,
None,
try_func,
None, /* this function can't return indirectly */
&[data],
then,
catch,
None,
None,
);
});

let func = unsafe { std::mem::transmute::<Function<'gcc>, RValue<'gcc>>(func) };

// Note that no invoke is used here because by definition this function
// can't panic (that's what it's catching).
let ret = bx.call(llty, None, None, func, &[try_func, data, catch_func], None, None);
let ret = bx.call(
llty,
None,
None,
func,
None, /*This function can't return indirectly*/
&[try_func, data, catch_func],
None,
None,
);
OperandValue::Immediate(ret).store(bx, dest);
}

Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_codegen_llvm/src/asm.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -511,9 +511,9 @@ pub(crate) fn inline_asm_call<'ll>(
assert!(catch_funclet.is_none());
bx.callbr(fty, None, None, v, inputs, dest.unwrap(), labels, None, None)
} else if let Some((catch, funclet)) = catch_funclet {
bx.invoke(fty, None, None, v, inputs, dest.unwrap(), catch, funclet, None)
bx.invoke(fty, None, None, v, None /*FIXME(FractalFir): I assume that inline assembly does not return via `PassMode::Indirect` - is this correct?*/, inputs, dest.unwrap(), catch, funclet, None)
} else {
bx.call(fty, None, None, v, inputs, None, None)
bx.call(fty, None, None, v, None /*FIXME(FractalFir): I assume that inline assembly does not return via `PassMode::Indirect` - is this correct?*/,inputs, None, None)
};

// Store mark in a metadata node so we can map LLVM errors
Expand Down
46 changes: 40 additions & 6 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 @@ -407,15 +407,26 @@ impl<'a, 'll, 'tcx> BuilderMethods<'a, 'tcx> for Builder<'a, 'll, 'tcx> {
fn_attrs: Option<&CodegenFnAttrs>,
fn_abi: Option<&FnAbi<'tcx, Ty<'tcx>>>,
llfn: &'ll Value,
indirect_return_pointer: Option<&'ll Value>,
args: &[&'ll Value],
then: &'ll BasicBlock,
catch: &'ll BasicBlock,
funclet: Option<&Funclet<'ll>>,
instance: Option<Instance<'tcx>>,
) -> &'ll Value {
// If this function returns indirectly(`PassMode::Indirect`),
// the `indirect_return_pointer` should be the first argument.
let args = match indirect_return_pointer {
None => args.to_vec(),
Some(sret_ptr) => {
let mut args = args.to_vec();
// Preappend the indirect return pointer.
args.insert(0, sret_ptr);
args
}
};
debug!("invoke {:?} with args ({:?})", llfn, args);

let args = self.check_call("invoke", llty, llfn, args);
let args = self.check_call("invoke", llty, llfn, &args);
let funclet_bundle = funclet.map(|funclet| funclet.bundle());
let mut bundles: SmallVec<[_; 2]> = SmallVec::new();
if let Some(funclet_bundle) = funclet_bundle {
Expand Down Expand Up @@ -1392,13 +1403,24 @@ impl<'a, 'll, 'tcx> BuilderMethods<'a, 'tcx> for Builder<'a, 'll, 'tcx> {
fn_attrs: Option<&CodegenFnAttrs>,
fn_abi: Option<&FnAbi<'tcx, Ty<'tcx>>>,
llfn: &'ll Value,
indirect_return_pointer: Option<&'ll Value>,
args: &[&'ll Value],
funclet: Option<&Funclet<'ll>>,
instance: Option<Instance<'tcx>>,
) -> &'ll Value {
// If this function returns indirectly(`PassMode::Indirect`),
// the `indirect_return_pointer` should be the first argument.
let args = match indirect_return_pointer {
None => args.to_vec(),
Some(sret_ptr) => {
let mut args = args.to_vec();
// Preappend the indirect return pointer
args.insert(0, sret_ptr);
args
}
};
debug!("call {:?} with args ({:?})", llfn, args);

let args = self.check_call("call", llty, llfn, args);
let args = self.check_call("call", llty, llfn, &args);
let funclet_bundle = funclet.map(|funclet| funclet.bundle());
let mut bundles: SmallVec<[_; 2]> = SmallVec::new();
if let Some(funclet_bundle) = funclet_bundle {
Expand Down Expand Up @@ -1442,7 +1464,19 @@ impl<'a, 'll, 'tcx> BuilderMethods<'a, 'tcx> for Builder<'a, 'll, 'tcx> {
funclet: Option<&Self::Funclet>,
instance: Option<Instance<'tcx>>,
) {
let call = self.call(llty, fn_attrs, Some(fn_abi), llfn, args, funclet, instance);
let call = self.call(
llty,
fn_attrs,
Some(fn_abi),
llfn,
None, /*
FIXME(FractalFir): Tail calls don't support indirect returns at the time of writing, but they will do so soon.
Once this support is added, the indirect return pointer ought to be passed here(if present).
*/
args,
funclet,
instance,
);
llvm::LLVMRustSetTailCallKind(call, llvm::TailCallKind::MustTail);

match &fn_abi.ret.mode {
Expand Down Expand Up @@ -1673,7 +1707,7 @@ impl<'a, 'll, 'tcx> Builder<'a, 'll, 'tcx> {
args: &[&'ll Value],
) -> &'ll Value {
let (ty, f) = self.cx.get_intrinsic(base_name.into(), type_params);
self.call(ty, None, None, f, args, None, None)
self.call(ty, None, None, f, None/* (FractalFir): at the time of writing, no LLVM intrinsic retruns data indirectly(via `sret`). So, this is always None.*/, args, None, None)
}

fn call_lifetime_intrinsic(&mut self, intrinsic: &'static str, ptr: &'ll Value, size: Size) {
Expand Down
Loading
Loading