search

llvm / llvm-project

The LLVM Project is a collection of modular and reusable compiler and toolchain technologies.
http://llvm.org
Other
27.9k stars 11.51k forks source link

Add `function_return` attribute and corresponding `-mfunction-return=` flag #54404

Open isanbard opened 2 years ago

isanbard commented 2 years ago

GCC supports afunction_return("choice") function attribute (https://gcc.gnu.org/onlinedocs/gcc/x86-Function-Attributes.html). This might be something clang should support.

function_return("choice")

On x86 targets, the function_return attribute causes the compiler to convert function return with choice. ‘keep’ keeps function return unmodified. ‘thunk’ converts function return to call and return thunk. ‘thunk-inline’ converts function return to inlined call and return thunk. ‘thunk-extern’ converts function return to external call and return thunk provided in a separate object file.

llvmbot commented 2 years ago

@llvm/issue-subscribers-clang-frontend

nickdesaulniers commented 2 years ago

cc @phoebewang

phoebewang commented 2 years ago

I googled but didn't find any information other that GCC document. Do we have any special requirment for it?

isanbard commented 2 years ago

This was implemented in GCC here:

https://gcc.gnu.org/git/?p=gcc.git;a=commit;h=45e140193876fab3bfb57779738d49ddc13fe9ed

phoebewang commented 2 years ago

Thanks @isanbard , my question was are there any projects using these options, so that we can evaluate whether we should implement it or simply ignore it.

isanbard commented 2 years ago

The Linux kernel is using it for some backends I believe. This is part of an attempt to get Clang even with GCC with regards to flags used by Linux.

nickdesaulniers commented 2 years ago

I only see -mfunction-return being used in arch/s390/Makefile.

nickdesaulniers commented 2 years ago

https://reviews.llvm.org/D129572

nickdesaulniers commented 2 years ago

Initial support has landed. Small fixups:

  1. https://reviews.llvm.org/D129691
  2. https://reviews.llvm.org/D129709

cc @uweigand @JonPsson for SystemZ support for the kernel. It might be worth a chat with your kernel team whether they really need (or prefer) CONFIG_EXPOLINE=y CONFIG_EXPOLINE_EXTERN=n (ie.-mfunction-return=thunk)? I doubt it, and will keep the impl simple to just support CONFIG_EXPOLINE=y CONFIG_EXPOLINE_EXTERN=y (ie. -mfunction-return=thunk-extern).

uweigand commented 2 years ago

cc @uweigand @JonPsson for SystemZ support for the kernel. It might be worth a chat with your kernel team whether they really need (or prefer) CONFIG_EXPOLINE=y CONFIG_EXPOLINE_EXTERN=n (ie.-mfunction-return=thunk)? I doubt it, and will keep the impl simple to just support CONFIG_EXPOLINE=y CONFIG_EXPOLINE_EXTERN=y (ie. -mfunction-return=thunk-extern).

Hi @nickdesaulniers, I talked to the kernel team, and they say CONFIG_EXPOLINE_EXTERN=y (i.e. -mfunction-return=thunk-extern) is always preferable, and sometimes even required (e.g. to support kpatch). The only reason it is not on by default is that it triggered a bug in some older GCC versions.

Therefore, I agree that if we're thinking about implementing expoline support in LLVM, support for olny -mfunction-return=thunk-extern would be fine.