This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH 3/3] or1k: gcc: initial support for openrisc
On 08/26/2018 03:18 PM, Stafford Horne wrote:
> yyyy-mm-dd Stafford Horne <shorne@gmail.com>
> Richard Henderson <rth@twiddle.net>
>
> gcc/ChangeLog:
>
> * common/config/or1k/or1k-common.c: New file.
> * config/or1k/*: New.
> * config.gcc (or1k*-*-*): New.
> * configure.ac (or1k*-*-*): New test for openrisc tls.
> * configure: Regenerated.
So I still want to do another looksie, but a few comments beyond what
Joseph has already pointed out.
Many functions are documented with "WORKER for xyz." kinds of comments.
Policy is each function should have a comment which describes what the
function does at a high level as well as the inputs/outputs.
Your port defines instruction scheduling, so please check that you're
emitting the proper barriers, particularly in your epilogue code. In
particular most ports need a barrier to prevent movement of register
restores past the stack adjustment when the frame pointer is in use.
Please consider using function descriptors rather than trampolines.
This allows you to make the stack non-executable at all times which is
good from a security standpoint. The downside is the indirect calling
mechanism has to change slightly to distinguish between a simple
indirect call and one through a function descriptor (usually by having a
low bit on to indicate the latter). GIven this is an ABI change, now is
probably the last opportunity to make this change.
I didn't see any provision for long vs short branches. How are branches
to targets within the same function, but which are out of the range that
can be encoded in a single instruction work? Similarly how is this
handled for function calls? Note handling this gets even more complex
with delay slots :(
I don't know if openrisc has any speculation capabilities. Please
define the SPECULATION_SAFE_VALUE hook appropriately for your
architecture. If you don't have speculation, define it to
speculation_safe_value_not_needed.
I don't see anything glaringly wrong, but would like to take another
looksie once the issues raised by Joseph and the function doc issue are
addressed.
Jeff
ps. 1/3 and 2/3 look fine.