This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH 1/3] [builtins] Generic support for __builtin_load_no_speculate()


On 01/09/2018 10:11 AM, Bill Schmidt wrote:
> 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.
Other than in the case where bounds_check is a compile-time constant I
don't see that the compiler is likely to drop the else clause above.  So
relying on the compiler to optimize away the redundant code doesn't seem
like a viable option.

However, if we relax the semantics on failval, then I think we get
enough freedom to generate the desired code on PPC.

> I'm just looking for a solution that works for everyone.
Likewise.  It'd be unfortunate if we end up with two distinct builtins
and the kernel buys have to write some level of abstraction on top of
them to select between them based on the target.


jeff


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]