__hardcfr_check_fail and BPF

__hardcfr_check_fail and BPF

[Jose E. Marchesi]

Hi Alex.

As you may know, in BPF we have to live (for now) with the constant pain
from being limited to functions whose arguments can be compiled to get
their arguments in five or less registers.

The recently introduced __hardcfr_check_fail in the run-time component
of hardcfr breaks the bpf-unknown-none build:

  ../../../libgcc/hardcfr.c: In function ‘__hardcfr_check_fail’:
  ../../../libgcc/hardcfr.c:210:1: error: too many function arguments for eBPF
    210 | __hardcfr_check_fail (size_t const blocks ATTRIBUTE_UNUSED,
        | ^~~~~~~~~~~~~~~~~~~~

It seems to me that __hardcfr_check_fail is only called from
__hardcfr_check, and compiled code is not instrumentalized with direct
calls to it.

If so, would it be possible to modify that function so it gets one less
argument? :)

Alternatively, we would need to disable the hardcfr from the BPF backend
and being able to define something in tm.h to inhibit building the
corresponding runtime in libgcc.  Would you be ok with having an #ifndef
DISABLE_LIBGCC_HARDCFR wrapping the stuff in that file?

Thanks.

Re: __hardcfr_check_fail and BPF

[Alexandre Oliva]

Hi,

On Oct 31, 2023, "Jose E. Marchesi" <jose.marchesi@oracle.com> wrote:

> As you may know, in BPF we have to live (for now) with the constant pain
> from being limited to functions whose arguments can be compiled to get
> their arguments in five or less registers.

Ugh.  I had no idea.

> The recently introduced __hardcfr_check_fail in the run-time component
> of hardcfr breaks the bpf-unknown-none build:

>   ../../../libgcc/hardcfr.c: In function ‘__hardcfr_check_fail’:
>   ../../../libgcc/hardcfr.c:210:1: error: too many function arguments for eBPF
>     210 | __hardcfr_check_fail (size_t const blocks ATTRIBUTE_UNUSED,
>         | ^~~~~~~~~~~~~~~~~~~~

Would it by any chance be enough to make it always_inline on bpf?

Failing that, the alternative I see is to pack some (or all) of the
arguments into a struct, and pass it by reference.  That would be plenty
of overhead for a slow path, and it could be made conditional for
targets that require it.

> It seems to me that __hardcfr_check_fail is only called from
> __hardcfr_check, and compiled code is not instrumentalized with direct
> calls to it.

Yeah, it's static, it's not even supposed to be visible.

I envisioned making it visible and forcing it out-of-line, so that one
could set a breakpoint on it in case of failures, but I haven't done
that (yet?).

I'm afraid dropping any of the arguments is not viable for it to do its
job, but calls to it could conceivably be replaced with a goto, if the
code is moved (manually inlined) into __hardcfr_check.

-- 
Alexandre Oliva, happy hacker            https://FSFLA.org/blogs/lxo/
   Free Software Activist                   GNU Toolchain Engineer
More tolerance and less prejudice are key for inclusion and diversity
Excluding neuro-others for not behaving ""normal"" is *not* inclusive

Re: __hardcfr_check_fail and BPF

[Jose E. Marchesi]

> Hi,
>
> On Oct 31, 2023, "Jose E. Marchesi" <jose.marchesi@oracle.com> wrote:
>
>> As you may know, in BPF we have to live (for now) with the constant pain
>> from being limited to functions whose arguments can be compiled to get
>> their arguments in five or less registers.
>
> Ugh.  I had no idea.
>
>> The recently introduced __hardcfr_check_fail in the run-time component
>> of hardcfr breaks the bpf-unknown-none build:
>
>>   ../../../libgcc/hardcfr.c: In function ‘__hardcfr_check_fail’:
>>   ../../../libgcc/hardcfr.c:210:1: error: too many function arguments for eBPF
>>     210 | __hardcfr_check_fail (size_t const blocks ATTRIBUTE_UNUSED,
>>         | ^~~~~~~~~~~~~~~~~~~~
>
> Would it by any chance be enough to make it always_inline on bpf?
>
> Failing that, the alternative I see is to pack some (or all) of the
> arguments into a struct, and pass it by reference.  That would be plenty
> of overhead for a slow path, and it could be made conditional for
> targets that require it.
>
>> It seems to me that __hardcfr_check_fail is only called from
>> __hardcfr_check, and compiled code is not instrumentalized with direct
>> calls to it.
>
> Yeah, it's static, it's not even supposed to be visible.
>
> I envisioned making it visible and forcing it out-of-line, so that one
> could set a breakpoint on it in case of failures, but I haven't done
> that (yet?).
>
> I'm afraid dropping any of the arguments is not viable for it to do its
> job, but calls to it could conceivably be replaced with a goto, if the
> code is moved (manually inlined) into __hardcfr_check.

Marking the function as always_inline would work with BPF.

I didn't thought of that because of that comment "When the verbose mode
is enabled, it also forces __hardcfr_debug_cfg (above) to be compiled
into an out-of-line function, that could be called from a debugger." but
now I realize it refers to __hardcfr_debug_cfg and not
__hardcfr_check_fail :)

Thank you.