On Mon, 9 Oct 2017 12:54:03 +0200
Halil Pasic <pa...@linux.vnet.ibm.com> wrote:
> On 10/09/2017 10:20 AM, Thomas Huth wrote:
> > On 04.10.2017 17:41, Halil Pasic wrote:
> >> +/* IO instructions conclude according this */
> >> +typedef struct IOInstEnding {
> >> + /*
> >> + * General semantic of cc codes of IO instructions is (brief):
> >> + * 0 -- produced expected result
> >> + * 1 -- status conditions were present or produced alternate
> >> result
> >> + * 2 -- ineffective, because busy with previously initiated
> >> function
> >> + * 3 -- ineffective, not operational
> >> + */
> >> + int cc;
> >> +} IOInstEnding;
> >
> > Why do you need a struct for this? Do you plan to extend it later? If
> > so, I think you should mention that in the patch description. If not,
> > please use a named enum or a "typedef unsigned int IOInstEnding" instead.
> >
> > Thomas
>
> We may, we may not. In the previous version we also had to support
> do end a certain instruction with an addressing exception, but this
> is going away in patch #3. Honestly I don't expect this being extended.
>
> I have other reasons for the struct. Type safety and clear semantics,
> and frankly at least for s390 and linux I don't see any downsides given
> what is written in the "zSeries ELF Application Binary Interface Supplement".
> Can you please explain to me what is the problem with using this struct, and
> what is the benefit switching to a unsigned int?
Honestly, I fail to see the benefit of using a struct here... it's just
a condition code, and while adding a comment what the various codes
mean for I/O instructions is a good idea, I think having to use a
IOInstEnding struct just renders the code less readable.
[I haven't had time to look at the rest of the patches yet.]
