[PATCH v8] Cygwin: pipe: Switch pipe mode to blocking mode by default

[Takashi Yano]

Hi Corinna,

On Thu, 31 Oct 2024 11:15:59 +0100
Corinna Vinschen wrote:
> Hi Takashi,
> 
> On Oct 31 17:36, Takashi Yano wrote:
> > Hi Corinna,
> > 
> > On Mon, 28 Oct 2024 12:57:05 +0100
> > Corinna Vinschen wrote:
> > > On Oct 28 20:25, Takashi Yano wrote:
> > > > Is the test case I used different from yours? Without the 2nd arg,
> > > > $ ./a.exe 40000
> > > > pipe capacity: 65536
> > > > write: writable 1, 40000 25536
> > > > write: writable 1, SIGALRM 24576 960
> > > > write: writable 0, SIGALRM -1 / Interrupted system call
> > > 
> > > This is the same testcase I pasted last week:
> > > 
> > >   $ ./x 40000
> > >   pipe capacity: 65536
> > >   write: writable 1, 40000 25536
> > >   write: writable 1, SIGALRM 24576 960
> > >   write: writable 0, SIGALRM 512 448
> > >   write: writable 0, SIGALRM 256 192
> > >   write: writable 0, SIGALRM 128 64
> > >   write: writable 0, SIGALRM 64 0
> > >   write: writable 0, SIGALRM -1 / Interrupted system call
> > > 
> > > So why does it not get into the last else case after calling
> > > pipe_data_available()?  Do you get a different return value
> > > from pipe_data_available()? If so, what and why?
> > 
> > I checked the behaviour in my environment.
> > __builtin_clzl(960) returns 54 in my environment.
> > So, result of
> > 	len1 = 1 << (31 - __builtin_clzl (avail));
> > is undefined. If I modify this to:
> > 	len1 = 1 << (63 - __builtin_clzl (avail));
> > I can get:
> > 
> > $ ./a.exe 40000 1
> > pipe capacity: 65536
> > write: writable 1, 40000 25536
> > write: writable 1, 24576 960
> > write: writable 0, 512 448
> > write: writable 0, 256 192
> > write: writable 0, 128 64
> > write: writable 0, 64 0
> > write: writable 0, -1 / Resource temporarily unavailable
> > 
> > with the commit 686e46ce7148 as well as with my v9 patch.
> > 
> > Could you please fix?
> 
> Yes, I will, but this is still puzzeling. While negative shift values
> are undefined in C, there's this:
> 
>   The Intel Pentium SAL instruction (generated by both gcc and Microsoft
>   C++ to evaluate left-shifts) only uses the bottom five bits of the
>   shift amount
> 
> The last 5 bits of 63 - 54 =   9 are 01001,
> the last 5 bits of 31 - 54 = -23 are 01001 as well.
> 
> I wrote a STC:
> 
> ------------------------------------
> #include <stdio.h>
> #include <stdlib.h>
> 
> int
> main (int argc, char **argv)
> {
>   ssize_t avail = atol (argv[1]);
> 
>   int x1 = 31 - __builtin_clzl (avail);
>   int x2 = 63 - __builtin_clzl (avail);
> 
>   printf ("%ld %d %u %u\n",
> 	  avail,
> 	  __builtin_clzl (avail),
> 	  1 << x1,
> 	  1 << x2);
>   return 0;
> }
> ------------------------------------
> 
> The workaround with x1 and x2 is necessary, otherwise gcc will
> fold the two expressions into a single sall instruction, even
> when building without optimization.
> 
> I can build the STC on Cygwin and with the Cygwin cross-compiler on
> Linux.  Both compilers generate identical assembler code.
> 
> In my environment the result is in both cases the same:
> 
>   $ ./clz-cyg 960
>   960 54 512 512
>   $ ./clz-lin 960
>   960 54 512 512
> 
> I get the same result, with and without -O2 (but then again, with -O2
> the sall instructions are folded into a single instruction again).
> 
> Do you get a different result?  Do you run this on an AMD CPU perhaps,
> and the AMDs implement the SAL instruction differently?

Please try this:

#include <stdio.h>
#include <stdlib.h>

#define PIPE_BUF 4096
int
main (int argc, char **argv)
{
  ssize_t avail = atol (argv[1]);
  unsigned long len1;

  if (avail < 1)
    return 0;
  if (avail == 1)
    len1 >>= 1;
  else if (avail >= PIPE_BUF)
    len1 = avail & ~(PIPE_BUF -1);
  else
    len1 = 1 << (31 - __builtin_clzl (avail));

  printf ("%ld %lu\n", avail, len1);
  return 0;
}

If the test case is compiled without optimization option,
$ ./a.exe 960
960 512

however, with -O2 option
$ ./a.exe 960
960 0

I am using gcc (GCC) 12.4.0 of cygwin gcc package.

It seems that the calcualtion of
    len1 = 1 << (31 - __builtin_clzl (avail));
is completely omitted.

In this case, avail == 1 or 1 < avail < 4096 for the last "else".
Therefore __builtin_clzl (avail) is always larger thatn 31.

I guess the compiler ommitted the undefined calculation.

-- 
Takashi Yano <takashi.yano@nifty.ne.jp>