This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH 1/3] [builtins] Generic support for __builtin_load_no_speculate()
On Jan 9, 2018, at 4:21 AM, Richard Earnshaw (lists) <Richard.Earnshaw@arm.com> wrote:
>
> On 08/01/18 16:01, Bill Schmidt wrote:
>> On Jan 8, 2018, at 8:06 AM, Richard Earnshaw (lists) <Richard.Earnshaw@arm.com> wrote:
>>>
>>> On 08/01/18 02:20, Bill Schmidt wrote:
>>>> Hi Richard,
>>>>
>>>> Unfortunately, I don't see any way that this will be useful for the ppc targets. We don't
>>>> have a way to force resolution of a condition prior to continuing speculation, so this
>>>> will just introduce another comparison that we would speculate past. For our mitigation
>>>> we will have to introduce an instruction that halts all speculation at that point, and place
>>>> it in front of all dangerous loads. I wish it were otherwise.
>>>
>>> So can't you make the builtin expand to (in pseudo code):
>>>
>>> if (bounds_check)
>>> {
>>> __asm ("barrier");
>>> result = *ptr;
>>> }
>>> else
>>> result = failval;
>>
>> Could, but this just generates unnecessary code for Power. We would instead generate
>>
>> __asm ("barrier");
>> result = *ptr;
>>
>> without any checks. We would ignore everything but the first argument.
>
> You can't do that with the builtin as it is currently specified as it
> also has a defined behaviour for the result it returns. You can,
> however, expand the code as normal RTL and let the optimizers remove any
> redundant code if they can make that deduction and you don't need the
> additional behaviour.
But that's my original point. "As currently specified" is overspecified for
our architecture, and expanding the extra code *hoping* it will go away
is not something I feel we should do. If our hopes are dashed, we end up
with yet worse performance. If we're going to use something generic
for everyone, then I argue that the semantics may not be the same for
all targets, and that this needs to be specified in the documentation.
I'm just looking for a solution that works for everyone.
Thanks,
Bill
>
> R.
>
>>
>> Thanks,
>> Bill
>>
>>>
>>> R.
>>>
>>>>
>>>> Thanks,
>>>> Bill
>>>>
>>>>> On Jan 4, 2018, at 7:58 AM, Richard Earnshaw <richard.earnshaw@arm.com> wrote:
>>>>>
>>>>>
>>>>> This patch adds generic support for the new builtin
>>>>> __builtin_load_no_speculate. It provides the overloading of the
>>>>> different access sizes and a default fall-back expansion for targets
>>>>> that do not support a mechanism for inhibiting speculation.
>>>>>
>>>>> * builtin_types.def (BT_FN_I1_CONST_VPTR_CONST_VPTR_CONST_VPTR_VAR):
>>>>> New builtin type signature.
>>>>> (BT_FN_I2_CONST_VPTR_CONST_VPTR_CONST_VPTR_VAR): Likewise.
>>>>> (BT_FN_I4_CONST_VPTR_CONST_VPTR_CONST_VPTR_VAR): Likewise.
>>>>> (BT_FN_I8_CONST_VPTR_CONST_VPTR_CONST_VPTR_VAR): Likewise.
>>>>> (BT_FN_I16_CONST_VPTR_CONST_VPTR_CONST_VPTR_VAR): Likewise.
>>>>> * builtins.def (BUILT_IN_LOAD_NO_SPECULATE_N): New builtin.
>>>>> (BUILT_IN_LOAD_NO_SPECULATE_1): Likewise.
>>>>> (BUILT_IN_LOAD_NO_SPECULATE_2): Likewise.
>>>>> (BUILT_IN_LOAD_NO_SPECULATE_4): Likewise.
>>>>> (BUILT_IN_LOAD_NO_SPECULATE_8): Likewise.
>>>>> (BUILT_IN_LOAD_NO_SPECULATE_16): Likewise.
>>>>> * target.def (inhibit_load_speculation): New hook.
>>>>> * doc/tm.texi.in (TARGET_INHIBIT_LOAD_SPECULATION): Add to
>>>>> documentation.
>>>>> * doc/tm.texi: Regenerated.
>>>>> * doc/cpp.texi: Document __HAVE_LOAD_NO_SPECULATE.
>>>>> * doc/extend.texi: Document __builtin_load_no_speculate.
>>>>> * c-family/c-common.c (load_no_speculate_resolve_size): New function.
>>>>> (load_no_speculate_resolve_params): New function.
>>>>> (load_no_speculate_resolve_return): New function.
>>>>> (resolve_overloaded_builtin): Handle overloading
>>>>> __builtin_load_no_speculate.
>>>>> * builtins.c (expand_load_no_speculate): New function.
>>>>> (expand_builtin): Handle new no-speculation builtins.
>>>>> * targhooks.h (default_inhibit_load_speculation): Declare.
>>>>> * targhooks.c (default_inhibit_load_speculation): New function.
>>>>> ---
>>>>> gcc/builtin-types.def | 16 +++++
>>>>> gcc/builtins.c | 99 ++++++++++++++++++++++++++
>>>>> gcc/builtins.def | 22 ++++++
>>>>> gcc/c-family/c-common.c | 164 ++++++++++++++++++++++++++++++++++++++++++++
>>>>> gcc/c-family/c-cppbuiltin.c | 5 +-
>>>>> gcc/doc/cpp.texi | 4 ++
>>>>> gcc/doc/extend.texi | 53 ++++++++++++++
>>>>> gcc/doc/tm.texi | 6 ++
>>>>> gcc/doc/tm.texi.in | 2 +
>>>>> gcc/target.def | 20 ++++++
>>>>> gcc/targhooks.c | 69 +++++++++++++++++++
>>>>> gcc/targhooks.h | 3 +
>>>>> 12 files changed, 462 insertions(+), 1 deletion(-)
>>>>>
>>>>> <0001-builtins-Generic-support-for-__builtin_load_no_specu.patch>