mbox series

[0/3,v2] Implement __builtin_speculation_safe_load

Message ID cover.1516199099.git.Richard.Earnshaw@arm.com
Headers show
Series Implement __builtin_speculation_safe_load | expand

Message

Richard Earnshaw (lists) Jan. 17, 2018, 2:55 p.m. UTC 
This patch series is version 2 for a patch to protect against
speculative use of a load instruction.  It's based on a roll-up of the
feedback from the first version.

What's changed since v1?

First, and foremost, the API has changed to make it possible to reduce
the amount of code that needs to be generated on architectures that
provide an unconditional speculation barrier.  Although the builtin
still requires the additional operands (and does some checking to
ensure that they aren't completely unreasonable), back-end expansion
can ignore them and simply emit a load instruction along with the
barrier operation..

Secondly, I've dropped the failval parameter.  With the new API, with
undefined behaviour for an out-of-bounds access, this paramter
no-longer makes sense.

Thirdly, based on off-list comments, I've dropped the ability to make
upper_bound NULL.  As a bounds value it was not particularly logical
and coding it requierd special handling in the back-end (wrong code
would be generated otherwise).  By removing it the bounds are now all
natural.  (NULL is still supported as a lower bound, nothing can be
less than address 0 --- at least on unsigned addressing-mode machines,
which appear to be all that GCC supports --- so it is a simple back-end
optimization to drop the check in this case.)

Finally, I've changed the name of the builtin.  This was mostly for
Arm's benefit since we already have some folk building code with the
old API and I need to be able to transition them cleanly to the new
one.  This is a one-off change, I'm not intending to support such
renames if we have to iterate again on this builtin before the initial
commit (ie I won't do it again, promise).

I've updated the documentation for the changes, please read that for
additional details (in patch 1).  Richi commented that he thought some
examples would help: I agree, but feel that putting them in
extend.texi with the builtin itself doesn't really fit with the rest
of that section - I think a web, or wiki page on the subject would be a
better bet.  That can be kept up-to-date more easily than the
documentation that comes with the compiler.

R.

Richard Earnshaw (3):
  [builtins] Generic support for __builtin_speculation_safe_load()
  [aarch64] Implement support for __builtin_speculation_safe_load
  [arm] Implement support for the de-speculation intrinsic

 gcc/builtin-types.def         |  16 +++++
 gcc/builtins.c                |  81 ++++++++++++++++++++++
 gcc/builtins.def              |  17 +++++
 gcc/c-family/c-common.c       | 152 ++++++++++++++++++++++++++++++++++++++++++
 gcc/c-family/c-cppbuiltin.c   |   5 +-
 gcc/config/aarch64/aarch64.c  |  81 ++++++++++++++++++++++
 gcc/config/aarch64/aarch64.md |  28 ++++++++
 gcc/config/arm/arm.c          | 100 +++++++++++++++++++++++++++
 gcc/config/arm/arm.md         |  40 ++++++++++-
 gcc/config/arm/unspecs.md     |   1 +
 gcc/doc/cpp.texi              |   4 ++
 gcc/doc/extend.texi           |  68 +++++++++++++++++++
 gcc/doc/tm.texi               |   9 +++
 gcc/doc/tm.texi.in            |   2 +
 gcc/target.def                |  34 ++++++++++
 gcc/targhooks.c               |  59 ++++++++++++++++
 gcc/targhooks.h               |   3 +
 17 files changed, 698 insertions(+), 2 deletions(-)

Comments

Joseph Myers Jan. 17, 2018, 5:15 p.m. UTC | #1 
This patch series seems to be missing testcases (generic and 
architecture-specific).  Generic ones should include testing the error 
cases that are diagnosed.

-- 
Joseph S. Myers
joseph@codesourcery.com