Re: gcc for m68332 isn't using bset and bclr

[Mike Stump <mrs at windriver dot com>]

> Date: Thu, 2 Mar 2000 01:35:59 -0600
> From: Clinton Popetz <cpopetz@cygnus.com>

> But even if there were a suitable pattern for this, it would fail on
> the above code, because the pointer is volatile, so general_operand
> and predicates that use it will return false during instruction
> combination.

This is a bug in the compiler which can be fixed.  At some point, I
will dust the patches off and submit them for inclusion.  They have
been submitted before, if people want to see them.  Search for
volatile_ok.

> You could add new patterns to m68k.md, something like this:

> (define_insn ""
>   [(set (match_operand:QI 0 "memory_operand" "+m")
>         (and:QI (match_dup 0)
>                 (match_operand:QI 1 "constant_mask_operand" "")))]
>   ""
>   "*
> {
>  /* figure out which bit is zero in the constant, and
>     return "bclr bit, %0" for that case. */ 
> }")

I like this one the best.

Note, you can currently see the compiler use bset/bclr with code like:

#if 0
#define CLEAR_BIT(a, b)  __asm volatile ("bclr.b %0, %1" : : "I"(b), "m" (a))
#define SET_BIT(a, b)    __asm volatile ("bset.b %0, %1" : : "I"(b), "m" (a))
#else
#define CLEAR_BIT(a, b)  ((a) &= ~ (1<<(b)))
#define SET_BIT(a, b)    ((a) |=   (1<<(b)))
#endif
 
#define volatile

int i = 1;

int
main(int argc, char *argv[])
{
  volatile unsigned char *ParPortData = (unsigned char*)0x00e01800;
  SET_BIT(*ParPortData, i);
  return 0;
}

gives:

        movel _i,d0
        bset d0,14686208

The #define volatile is to work around the bug mentioned above.  The
use of int i is to let the compiler `see' the operation (to keep it
from constant folding it).  To get it to generate the code you expect,
we can add:

Doing diffs in .:
*** ./m68k.md.~1~	Mon Feb 28 20:34:16 2000
--- ./m68k.md	Thu Mar  2 11:17:16 2000
***************
*** 3748,3753 ****
--- 3748,3777 ----
    "!TARGET_5200"
    "and%.w %1,%0")
  
+ (define_insn ""
+   [(set (match_operand:QI 0 "memory_operand" "+m")
+         (ior:QI (match_dup 0)
+                 (match_operand:QI 1 "const_int_operand" "n")))]
+   "exact_log2 (INTVAL (operands[1]) & 0xff) >= 0"
+   "*
+ {
+     CC_STATUS_INIT;
+     operands[1] = GEN_INT (exact_log2 (INTVAL (operands[1]) & 0xff));
+     return \"bset %1,%0\";
+ }")
+ 
+ (define_insn ""
+   [(set (match_operand:QI 0 "memory_operand" "+m")
+         (and:QI (match_dup 0)
+                 (match_operand:QI 1 "const_int_operand" "n")))]
+   "exact_log2 (~INTVAL (operands[1]) & 0xff) >= 0"
+   "*
+ {
+     CC_STATUS_INIT;
+     operands[1] = GEN_INT (exact_log2 (~INTVAL (operands[1]) & 0xff));
+     return \"bclr %1,%0\";
+ }")
+ 
  (define_insn "andqi3"
    [(set (match_operand:QI 0 "general_operand" "=m,d")
  	(and:QI (match_operand:QI 1 "general_operand" "%0,0")
--------------

I tested this and it generates what looks to be the right code.  Try
it out and let us know.  The code I tested it on was:

#if 0
#define CLEAR_BIT(a, b)  __asm volatile ("bclr.b %0, %1" : : "I"(b), "m" (a))
#define SET_BIT(a, b)    __asm volatile ("bset.b %0, %1" : : "I"(b), "m" (a))
#else
#define CLEAR_BIT(a, b)  ((a) = (~ (1<<(b))) & (a))
#define SET_BIT(a, b)    ((a) |=   (1<<(b)))
#endif
 
#define volatile

int
main(int argc, char *argv[])
{
  int i = 4;
  volatile unsigned char *ParPortData = (unsigned char*)0x00e01800;
  SET_BIT(*ParPortData, i);
  return 0;
}

varying the 4 and SET/CLEAR.  The size of the resulting code is the
same.  I don't know enough about the CPU conflicts (those that might
prevent overlap) and instruction speed to know if the above is worse
for any reason than the and/or that it would have generated before.