DPDK patches and discussions
 help / color / mirror / Atom feed
From: Thomas Monjalon <thomas@monjalon.net>
To: "Ananyev, Konstantin" <konstantin.ananyev@intel.com>
Cc: Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com>,
	"dev@dpdk.org" <dev@dpdk.org>,
	"aconole@redhat.com" <aconole@redhat.com>, nd <nd@arm.com>
Subject: Re: [dpdk-dev] [PATCH] test/ring: code rework to reduce compilation time
Date: Mon, 04 May 2020 11:03:06 +0200	[thread overview]
Message-ID: <2150775.ElGaqSPkdT@thomas> (raw)
In-Reply-To: <BYAPR11MB33012D89AC7524B941AB15E19AAA0@BYAPR11MB3301.namprd11.prod.outlook.com>

30/04/2020 16:43, Ananyev, Konstantin:
> 
> Hi Honnappa,
>  
> > Hi Konstantin,
> > 	I like the way the tests are organized and it looks good.
> > 
> > I am just wondering about the way it is being tested here. The intent to write the test cases the way they are currently is to mimic how the
> > APIs would be used mostly. IMO, the APIs would be used with a constant value for element size so that the compiler will throw away the
> > unwanted code (in the functions where the actual copy is being done).
> > 
> > With your method here, it looks to me like all the branches in the copy functions are kept and the branch decisions are done at run time.
> > Is  my understanding correct?
> 
> You mean branching on esize[] values?
> Actually from what I've seen that happens for both cases:
> before and after the patch (gcc 7.3 -O3).
> 
> Main intention in my changes was to avoid using test_ring_enqueue/test_ring_dequeue,
> as it seems too many branches here and it takes compiler a lot of effort to resolve all
> of them at compile time.
> So I replaced it with  array of function pointers (test_enqdeq_impl[]) and iterating over it.
> That way compiler knows straightway which function to use.

In case we choose this solution, please make a v2 including such explanations.




  parent reply	other threads:[~2020-05-04  9:03 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-04-29 17:57 Konstantin Ananyev
2020-04-29 18:19 ` Honnappa Nagarahalli
2020-04-29 22:10 ` Aaron Conole
2020-04-30  1:57 ` Honnappa Nagarahalli
2020-04-30 14:43   ` Ananyev, Konstantin
2020-05-01 17:48     ` Honnappa Nagarahalli
2020-05-05 11:59       ` Ananyev, Konstantin
2020-05-05 14:17         ` Honnappa Nagarahalli
2020-05-05 14:27           ` Ananyev, Konstantin
2020-05-05 18:13             ` Honnappa Nagarahalli
2020-05-05 18:57               ` Ananyev, Konstantin
2020-05-08  5:32                 ` Honnappa Nagarahalli
2020-05-08 10:04                   ` Ananyev, Konstantin
2020-05-09  0:33                     ` Honnappa Nagarahalli
2020-05-11 11:35                       ` Ananyev, Konstantin
2020-05-13  0:55                         ` Honnappa Nagarahalli
2020-05-13 11:20                           ` Ananyev, Konstantin
2020-05-04  9:03     ` Thomas Monjalon [this message]
2020-07-02 20:47 ` Honnappa Nagarahalli
2020-07-03 10:29   ` David Marchand

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=2150775.ElGaqSPkdT@thomas \
    --to=thomas@monjalon.net \
    --cc=Honnappa.Nagarahalli@arm.com \
    --cc=aconole@redhat.com \
    --cc=dev@dpdk.org \
    --cc=konstantin.ananyev@intel.com \
    --cc=nd@arm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).