Re: h8300: less optimal (buggy?) compiler output with last build

[Jeffrey A Law <law at cygnus dot com>]

  In message <399C40C1.B01B00D@cygnus.co.uk>you write:
   > I took a look at this problem. It's target independent and the problem (
  > extranous read from volatile) will occur assignment to a volatile variable
  > within a GCC statement expression (i.e. ({...})). You are asking where is
  > the statement expression in the above code. Well it's added by the C++
  > front-end when inlining Watchdog.
  > 
  > Here's a C testcase which also shows the bug.
  > -----------------------------------------------------
  > extern volatile unsigned char TCW;
  > 
  > #define  WD()   \ 
  > ({              \ 
  >   TCW = 0;      \
  >   1;            \
  > })
  > 
  > int test(void)
  > {
  >    return WD();
  > }
  > ------------------------------------------------------
  > 
  > This is what I get compiling it with -O0 -S -g0 -fomit-frame-pointer
  > altough you get the same extra "movzbl TCW,%eax" no matter what options
  > are used.
  >  
  > ------------------------------------------------------
  >         .file   "test.c"
  >         .version        "01.01"
  > gcc2_compiled.:
  > .text
  >         .align 16
  > .globl test
  >         .type    test,@function
  > test:
  >         movb    $0, TCW
  >         movzbl  TCW, %eax
  >         movl    $1, %eax
  >         ret
  > .Lfe1:
  >         .size    test,.Lfe1-test
  >         .ident  "GCC: (GNU) 2.96 20000817 (experimental)"
  > 
  > ------------------------------------------------------
  > 
  > The extraneous "movzbl TCW,%eax" is coming from the copy_to_reg in
  > this block of code at the end of store_expr.
  > 
  >   /* If we don't want a value, return NULL_RTX.  */
  >   if (! want_value)
  >     return NULL_RTX;
  > 
  >   /* If we are supposed to return TEMP, do so as long as it isn't a MEM.
  >      ??? The latter test doesn't seem to make sense.  */
  >   else if (dont_return_target && GET_CODE (temp) != MEM)
  >     return temp;
  > 
  >   /* Return TARGET itself if it is a hard register.  */
  >   else if (want_value && GET_MODE (target) != BLKmode
  >            && ! (GET_CODE (target) == REG
  >                  && REGNO (target) < FIRST_PSEUDO_REGISTER))
  >     return copy_to_reg (target);
  > 
  >   else
  >     return target;
  > }
  > 
  > What I don't understand is why the copy_to_reg is needed! can't it just
  > return target? (thus)
  > 
  > 
  >   /* If we don't want a value, return NULL_RTX.  */
  >   if (! want_value)
  >     return NULL_RTX;
  > 
  >   /* If we are supposed to return TEMP, do so as long as it isn't a MEM.
  >      ??? The latter test doesn't seem to make sense.  */
  >   else if (dont_return_target && GET_CODE (temp) != MEM)
  >     return temp;
  >   else
  >     return target;
  > }
After a wander through the old dusty GCC code from the early/mid 90s I
found the code was added by the following change:

Sat Feb 26 07:14:04 1994  Richard Kenner  (kenner@vlsi1.ultra.nyu.edu)

        * expr.c (store_expr): In promoted TARGET case, if EXP is a
        volatile MEM and WANT_VALUE is nonzero, only reference the MEM once.
        Return TARGET if it is a hard register; clean up return code.

Obviously the whole point of this patch was to avoid multiple references
to MEM.

The way it did this was it detected that temp was a volatile MEM and
arranged to copy it into a register earlier in the code.  That way 
target would be a register by the time we got to the return code.

Presumably we can do something similar for whatever path we're taking
through store_expr for this case.

It might help the analysis some if you showed the tree node for exp,
incoming value of target and want_value.

jeff
jeff