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 V2 2/8] bpf: new GCC port


On 8/20/19 3:14 PM, Jose E. Marchesi wrote:
> 
> Hi Jeff.
> 
>     > This patch adds a port for the Linux kernel eBPF architecture to GCC.
>     > 
>     > ChangeLog:
>     > 
>     >   * configure.ac: Support for bpf-*-* targets.
>     >   * configure: Regenerate.
>     > 
>     > contrib/ChangeLog:
>     > 
>     >   * config-list.mk (LIST): Disable go in bpf-*-* targets.
>     > 
>     > gcc/ChangeLog:
>     > 
>     >   * config.gcc: Support for bpf-*-* targets.
>     >   * common/config/bpf/bpf-common.c: New file.
>     >   * config/bpf/t-bpf: Likewise.
>     >   * config/bpf/predicates.md: Likewise.
>     >   * config/bpf/constraints.md: Likewise.
>     >   * config/bpf/bpf.opt: Likewise.
>     >   * config/bpf/bpf.md: Likewise.
>     >   * config/bpf/bpf.h: Likewise.
>     >   * config/bpf/bpf.c: Likewise.
>     >   * config/bpf/bpf-protos.h: Likewise.
>     >   * config/bpf/bpf-opts.h: Likewise.
>     >   * config/bpf/bpf-helpers.h: Likewise.
>     >   * config/bpf/bpf-helpers.def: Likewise.
>     So I think various folks have already mentioned the configure rebuild
>     issues, formatting and other stuff.  I'm going to try to keep them all
>     in mind so that I don't duplicate anything.  If I do duplicate someone's
>     comment, apologies in advance.
>     
>     At a high level I realize there's lots of things not supported due to
>     the restricted environment it'll ultimately be used in.  However, you
>     might want to consider extensions that would allow larger portions of
>     the gcc testsuite to run and some kind of user mode simulator so that
>     you can reasonably test the target.  Not a requirement, but could be
>     useful (from experience :-)
> 
> I agree to both regards.
> 
> I have been thinking about Segher's suggestion on providing options to
> lift some of the limitations, for compiler testing.  Unfortunately, many
> of the restrictions are deeply rooted in the design of the
> architecture... or the other way around.  Finding sane ways to implement
> these extensions will be fun :)
Hell, it's a virtual architecture.  I'd just make up new instructions
for the missing functionality, make them dependent on an flag.  I think
the PRU is in a similar position and uses that approach.  PTX might have
as well.


> 
> As for the simulator, I have one, along with an initial GDB port... but
> it doesn't work very well due to a particularly nasty bug in CGEN.  I
> have a patch that seems to fix it but, as everything that touches cgen's
> ifield handling code, it is difficult to be 100% sure about that, and I
> also need to adapt some of the other existing cgen-based ports...  so it
> will take a while before I have something that can run the GCC
> testsuite.
ACK.

>     Curious about the motivation on the loop unrolling stuff.  In general we
>     discourage targets from mucking around with the default
>     flags/optimizations, but it is sometimes the right thing to do.
> 
> The kernel verifier doesn't allow backward jumps.
Oh yea, I should have remembered that.  I think it came up in a
systemtap and/or ebpf+systemtap discussion at some point.

> 
> This may change at some point.  There is much discussion among the
> kernel hackers in whether it is possible to allow bounded loops in a
> safe way.  In that case, some of the restrictions may be lifted.
ACK.  It's an interesting problem.  Would it help if we could annotate
loops with bound information?  Not sure how to preserve that from gimple
down to assembly, but it's worth pondering.

>     Does this get called often?  If so the linear search could end up being
>     expensive from a compile-time standpoint.
> 
> It gets called per function call to a symbol with the form
> __builtin_bpf_helper_*...  you think it is worth of a hash?
Hard to tell.  Maybe leave it for now and revisit post integration and
real world feedback.

>     Is the stack limit likely to change?  Would a param work better here
>     which would allow us to accommodate such a change without having to
>     re-release GCC?
>     
> It will probably be increased at some point.  Using a param sounds like
> a good idea.  However...
> 
> The stack limit is associated with kernel version.  I guess we can just
> set the appropriate defaults in bpf_option_override if we make it
> variable, in case the user didn't specify a --param for it, so no
> problem.
> 
> Also, if we allow the user to specify a stack frame bigger than 0x7fff,
> bpf_expand_prologue will break.  Probably in that case we want to detect
> this, warn and truncate to the -mkernel's default, also in
> bpf_option_override.
> 
> Does that sound reasonable?
It does.  I think PARAMS have the ability to enforce a min/max and
specify a default.  So set a default to 512 since that's what works
everywhere and if the kernel bumps up, folks can just use the param to
allow more stack space.

> 
>     > diff --git a/gcc/config/bpf/bpf.h b/gcc/config/bpf/bpf.h
>     > +
>     > +/**** Debugging Info ****/
>     > +
>     > +/* We cannot support DWARF2 because of the limitations of eBPF.  */
>     > +#define DBX_DEBUGGING_INFO
>     Umm, we're trying to get rid of DBX_DEBUGGING_INFO.  I'd rather not add
>     another user at this point.  How tough would it be to support dwarf?
> 
> Yes relying on stabs sucks.
> 
> The main problem I found is that it is not possible to define a CFA, nor
> to unwind frames in any way.  Given these limitations, is it still
> possible to make GCC emit minimally useful DWARF, with locations and
> such?  That would be great.
I thought we had that ability in the past.  It may have bitrotted since
most of our targets have moved to supporting CFA and dwarf2.


>     Hmm, what happens if you need to reload something from a constant
>     address?  You can't call gen_reg_rtx once register allocation has
>     started.  THe case where you need a scratch register really feels like
>     you need to be defining secondary reloads.
> 
> I really have to think about this.  Richard's comment about the
> possibility of not considering constant addresses legit already made me
> ponder whether it would be better to use a different strategy here.
One approach would be to not allow them initially.  You can then add
them back with the necessary secondary reload support at a later time.

This is one of those areas where being able to run the testsuite really
helps :-)


Jeff


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