Re: [PATCH V6 2/3] sim: eBPF simulator

Jose E. Marchesi
2020-08-04T15:32:22Z

>> This patch introduces the basics of an instruction-simulator for eBPF.
>> The simulator is based on CGEN.
>>
>> sim/ChangeLog:
>>
>> 2020-08-03 Jose E. Marchesi <jose.marchesi@oracle.com>
>> David Faust <david.faust@oracle.com>
>>
>> * configure.tgt (sim_arch): Add entry for bpf-*-*.
>> * configure: Regenerate.
>> * MAINTAINERS: Add maintainer for the BPF simulator.
>> * bpf/Makefile.in: New file.
>> * bpf/bpf-helpers.c: Likewise.
>> * bpf/bpf-helpers.def: Likewise.
>> * bpf/bpf-helpers.h: Likewise.
>> * bpf/bpf-sim.h: Likewise.
>> * bpf/bpf.c: Likewise.
>> * bpf/config.in: Likewise.
>> * bpf/configure.ac: Likewise.
>
> You need to regenerate bpf/configure and commit it.

I removed that thunk from the patch due to the size limit. It is in.

>> * bpf/decode.h: Likewise.
>> * bpf/eng.h: Likewise.
>> * bpf/mloop.in: Likewise.
>> * bpf/sim-if.c: Likewise.
>> * bpf/sim-main.h: Likewise.
>> * bpf/traps.c: Likewise.
>> * bpf/configure: Generate.
>> * bpf/aclocal.m4: Likewise.
>>
>> sim/testsuite/ChangeLog:
>>
>> 2020-08-03 David Faust <david.faust@oracle.com>
>> Jose E. Marchesi <jose.marchesi@oracle.com>
>>
>> * configure: Regenerate.
>> * sim/bpf/allinsn.exp: New file.
>> * sim/bpf/alu.s: Likewise.
>> * sim/bpf/alu32.s: Likewise.
>> * sim/bpf/endbe.s: Likewise.
>> * sim/bpf/endle.s: Likewise.
>> * sim/bpf/jmp.s: Likewise.
>> * sim/bpf/jmp32.s: Likewise.
>> * sim/bpf/ldabs.s: Likewise.
>> * sim/bpf/mem.s: Likewise.
>> * sim/bpf/mov.s: Likewise.
>> * sim/bpf/testutils.inc: Likewise.
>> * sim/bpf/xadd.s: Likewise.
>> ---
>> gdb/ChangeLog | 4 +
>> gdb/configure.tgt | 1 +
>
> Make sure you include the gdb/ChangeLog entry in the commit message.

Ok.

>> +# Note the following files are generated in objdir, not srcdir.
>> +
>> +stamp-mloop: stamp-mloop-le stamp-mloop-be
>> +
>> +stamp-mloop-le: $(srcdir)/../common/genmloop.sh mloop.in Makefile
>> + $(SHELL) $(srccom)/genmloop.sh -shell $(SHELL) \
>> + -mono -scache -prefix bpfbf_ebpfle -cpu bpfbf \
>> + -infile $(srcdir)/mloop.in
>> + $(SHELL) $(srcroot)/move-if-change eng.hin eng-le.h
>> + $(SHELL) $(srcroot)/move-if-change mloop.cin mloop-le.c
>> + touch $@
>> +mloop-le.c eng-le.h: stamp-mloop-le
>> + @true
>> +
>> +stamp-mloop-be: $(srcdir)/../common/genmloop.sh mloop.in Makefile
>> + $(SHELL) $(srccom)/genmloop.sh -shell $(SHELL) \
>> + -mono -scache -prefix bpfbf_ebpfbe -cpu bpfbf \
>> + -infile $(srcdir)/mloop.in
>> + $(SHELL) $(srcroot)/move-if-change eng.hin eng-be.h
>> + $(SHELL) $(srcroot)/move-if-change mloop.cin mloop-be.c
>> + touch $@
>> +mloop-be.c eng-be.h: stamp-mloop-be
>> + @true
>
> When testing the build I ran into issues here while trying to perform
> a parallel build as both these rules generate the files eng.hin and
> mloop.cin.
>
> I know this problem is not unique to this target, but it would be nice
> if we didn't introduce more places where the parallel build is broken.
>
> The _correct_ solution would be to update genmloop.sh so that the
> output files can be named. However, I'm aware that this is an
> unreasonable expectation, so could you at least combine the be and le
> generation into a single rule 'stamp-all-mloop' please, and add a
> comment explaining why. This at least will work around the problem
> for now.

Regarding genmloop.sh, I just noticed the script may be already
supporting this?

-outfile-suffix) shift ; outsuffix=$1 ;;

I'm using it now like this, and seems to work well with -j:

# Note the following files are generated in objdir, not srcdir.

stamp-mloop: stamp-mloop-le stamp-mloop-be

stamp-mloop-le: $(srcdir)/../common/genmloop.sh mloop.in Makefile
$(SHELL) $(srccom)/genmloop.sh -shell $(SHELL) \
-mono -scache -prefix bpfbf_ebpfle -cpu bpfbf \
-infile $(srcdir)/mloop.in -outfile-suffix -le
$(SHELL) $(srcroot)/move-if-change eng-le.hin eng-le.h
$(SHELL) $(srcroot)/move-if-change mloop-le.cin mloop-le.c
touch $@
mloop-le.c eng-le.h: stamp-mloop-le
@true

stamp-mloop-be: $(srcdir)/../common/genmloop.sh mloop.in Makefile
$(SHELL) $(srccom)/genmloop.sh -shell $(SHELL) \
-mono -scache -prefix bpfbf_ebpfbe -cpu bpfbf \
-infile $(srcdir)/mloop.in -outfile-suffix -be
$(SHELL) $(srcroot)/move-if-change eng-be.hin eng-be.h
$(SHELL) $(srcroot)/move-if-change mloop-be.cin mloop-be.c
touch $@
mloop-be.c eng-be.h: stamp-mloop-be
@true

> With these issues resolved I'm happy for this to go in.

Thanks!