public inbox for gcc-cvs@sourceware.org
help / color / mirror / Atom feed
* [gcc/devel/rust/master] Merge #1630
@ 2022-12-01  8:12 Thomas Schwinge
  0 siblings, 0 replies; only message in thread
From: Thomas Schwinge @ 2022-12-01  8:12 UTC (permalink / raw)
  To: gcc-cvs

https://gcc.gnu.org/g:738df996e0b5ce53b8681cdc34e64cf0f2b37020

commit 738df996e0b5ce53b8681cdc34e64cf0f2b37020
Merge: 71b8beb150d fc59d137491
Author: bors[bot] <26634292+bors[bot]@users.noreply.github.com>
Date:   Wed Nov 30 09:46:51 2022 +0000

    Merge #1630
    
    1630: Cleanup builtin handling r=CohenArthur a=CohenArthur
    
    This PR adds proper GCC-like handling of builtins, which allows us to not re-define all the builtins we use and fight with declaring the types each time.
    
    This is also safer and works better. Sadly, this has unearthed some problems in our handling of builtins, specifically the ones I've added recently (atomic ones and data prefetching). These are the two issues that remain for this PR to be in a nice state which does not cause regressions:
    
    1. `atomic_{store, load}` (and maybe cmp_xchg) do not work on signed integers. I'm currenty working around this by simply using `u32`s instead of `i32`s for the tests, but that's not a valid solution. These intrinsics need to work on all integer types and I will thus need to add some conversions from `i*` to `u*` before calling the intrinsics. The upside is that with these cleanups we should now be able to handle `*size` types easily and cleanly for those intrinsics
    2. `__builtin_prefetch()` requires the `locality` argument (third argument) to be a const value. While LLVM will let you build the function and maybe error out later down the line, GCC does not let you pass non compile time known values as locality arguments. Because we are trying to build the following intrinsic:
    
    ```rust
    fn prefetch_read_data<T>(src: *const T, locality: i32) {
        __builtin_prefetch(src, 1, locality);
    }
    ```
    
    we cannot know that the `locality` arg is a compile-time constant. There are two ways to go around this problem:
    
    a. Try to constant fold the `locality` argument. If this works, it's a compile time constant and we can keep going
    b. Somehow map a generic intrinsic directly to a GCC one and inserting a new argument. So instead of generating something like
    
    ```rust
    fn prefetch_read_data<i32>(src: *const i32, locality: i32) {
        __builtin_prefetch(src, 0, locality)
    }
    ```
    
    we'd swap the call to `prefetch_read_data::<i32>(src, locality)` with `__builtin_prefetch(src, 0, locality)` and enforce `locality` being a compile time constant.
    
    Edited because dynamically dispatching a *compiler hint* does not make any sense at all.
    
    Co-authored-by: Arthur Cohen <arthur.cohen@embecosm.com>

Diff:

 gcc/rust/backend/rust-builtins.cc                  | 350 ++++++++++++++-------
 gcc/rust/backend/rust-builtins.h                   | 118 ++++++-
 gcc/rust/backend/rust-compile-intrinsic.cc         |  41 ++-
 gcc/rust/rust-gcc.cc                               |   8 +-
 gcc/testsuite/rust/compile/torture/intrinsics-4.rs |   2 +-
 gcc/testsuite/rust/execute/torture/atomic_load.rs  |   4 +-
 gcc/testsuite/rust/execute/torture/atomic_store.rs |   4 +-
 7 files changed, 389 insertions(+), 138 deletions(-)

^ permalink raw reply	[flat|nested] only message in thread

only message in thread, other threads:[~2022-12-01  8:12 UTC | newest]

Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-12-01  8:12 [gcc/devel/rust/master] Merge #1630 Thomas Schwinge

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).