Re: [POC 3/5] sframe: add new SFrame library

From: Steven Rostedt <rostedt@goodmis.org>
To: Peter Zijlstra <peterz@infradead.org>
Cc: Indu Bhagat <indu.bhagat@oracle.com>,
	linux-toolchains@vger.kernel.org, daandemeyer@meta.com,
	andrii@kernel.org, kris.van.hees@oracle.com,
	elena.zannoni@oracle.com, nick.alcock@oracle.com
Subject: Re: [POC 3/5] sframe: add new SFrame library
Date: Tue, 2 May 2023 11:22:12 -0400	[thread overview]
Message-ID: <20230502112212.67b251eb@gandalf.local.home> (raw)
In-Reply-To: <20230502104125.GN1597476@hirez.programming.kicks-ass.net>

On Tue, 2 May 2023 12:41:25 +0200
Peter Zijlstra <peterz@infradead.org> wrote:

> > +{
> > +	/* Type size of the start_addr in an FRE. */
> > +	size_t saddr_tsize = 0;
> > +
> > +	switch (fre_type) {
> > +	case SFRAME_FRE_TYPE_ADDR1:
> > +		saddr_tsize = sizeof(uint8_t);
> > +		break;
> > +	case SFRAME_FRE_TYPE_ADDR2:
> > +		saddr_tsize = sizeof(uint16_t);
> > +		break;
> > +	case SFRAME_FRE_TYPE_ADDR4:
> > +		saddr_tsize = sizeof(uint32_t);
> > +		break;
> > +	default:
> > +		/* No other value is expected. */
> > +		break;
> > +	}
> > +	return saddr_tsize;  
> 
> 	return 1 << fre_type;
> 
> was too complicated?

I would argue that the above is easier for the reviewer, but yes, the
"1 << fre_type" works too, and is more efficient, but requires a comment
that explains it.

It would also require a comment stating the dependency to this by the enum
definition.

-- Steve