> -----Original Message-----
> From: Eelco Chaudron <[email protected]>
> Sent: Thursday, June 30, 2022 12:44 PM
> To: Van Haaren, Harry <[email protected]>
> Cc: Finn, Emma <[email protected]>; Stokes, Ian <[email protected]>;
> [email protected]
> Subject: Re: [PATCH v7 02/11] odp-execute: Add function pointers to
> odp-execute
> for different action implementations.
>
>
>
> On 30 Jun 2022, at 10:16, Van Haaren, Harry wrote:
>
> >> -----Original Message-----
> >> From: Eelco Chaudron <[email protected]>
> >> Sent: Thursday, June 23, 2022 4:38 PM
> >> To: Finn, Emma <[email protected]>
> >> Cc: Stokes, Ian <[email protected]>; Van Haaren, Harry
> >> <[email protected]>; [email protected]
> >> Subject: Re: [PATCH v7 02/11] odp-execute: Add function pointers to
> >> odp-execute
> >> for different action implementations.
> >>
> >> On 14 Jun 2022, at 13:57, Emma Finn wrote:
> >
> > <snip patch contents>
> >
> >>> --
> >>> 2.32.0
> >>
> >>
> >> General discussion item from v6:
> >
> > Thanks Eelco for the detailed review.
> >
> >
> >> EC> I have another general architecture question? What if for a specific
> >> packet
> (or
> >> batch of packets) the SIMD implementation is not able to process the
> packet/batch?
> >> EC> How can we handle this in a uniform way? Can you give this some thought
> and
> >> add support?
> >>
> >> EF> We support 100% of the action (regardless of input packet, we perform
> >> same
> >> action as scalar actions code). Autovalidation ensures the modifications
> >> are
> >> identical.
> >> EF> As a result there is no value in a "fallback to scalar" backup, as
> >> this must never
> >> occur.
> >>
> >> What I'm trying to say is, that in certain cases, your SIMD specific code
> >> might not
> be
> >> able to optimize the action handling, and it might want to fall back to
> >> the scaler
> >> implementation.
> >
> > Today, the AVX512 implementation in this patchset will *always* be able to
> > handle
> the action.
> > There is no requirement for scalar fallback.
>
> Well actually there is a need, but don’t worry I fixed/added it to my patch
> 10 review.
>
> Take a look at the quick fix here, https://mail.openvswitch.org/pipermail/ovs-
> dev/2022-June/395068.html, where some scalar functions get called.
Specifically, you mean this bit?
@@ -411,6 +412,13 @@ action_avx512_set_masked(struct dp_packet_batch *batch
OVS_UNUSED,
if (avx512_impl.set_masked_funcs[attr_type]) {
avx512_impl.set_masked_funcs[attr_type](batch, a);
+ } else {
+ struct dp_packet *packet;
+ a = nl_attr_get(a);
+
+ DP_PACKET_BATCH_FOR_EACH (i, packet, batch) {
+ odp_execute_masked_set_action(packet, a);
+ }
}
I don't see that as a *fallback* because something couldn't be handled,
but a "use-scalar" because the AVX512 isn't implemented (func ptr = NULL).
It's not the case that the AVX512 couldn’t handle a specific input packet here;
the case is that Action has SubActions, and all are not implemented yet.
Most Actions do NOT have SubActions (like SET_MASKED has). I think it's OK to
handle
the "use-scalar" in these few cases.
My main concern was around requirements hypothetical OpenCL implementations.
To handle in a generic way at the Action level (instead of SubAction like
above) seems
to over-complicate things to me...
Input would be useful to quickly find path forward and get that built & merged.
Is the above OK for 2.18, and refactor/rework if required for 2.19/onwards?
<snip>
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
