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

[Clinton Popetz <cpopetz at cygnus dot com>]

On Thu, Mar 02, 2000 at 12:52:01PM +0800, Petter Reinholdtsen wrote:
> 
> I'm using GCC 2.95.2 as a cross compiler for the m68332 micro
> controller.
> 
> The compiler does not seem to use the bset and bclr instructions
> available on the processor.  The following test program should
> demonstrate the problem.
> 
>   #define CLEAR_BIT(a, b)  ((a) &= ~ (1<<(b)))
>   #define SET_BIT(a, b)    ((a) |=   (1<<(b)))

>     volatile unsigned char *ParPortData = (unsigned char*)0x00e01800;
>     CLEAR_BIT(*ParPortData, 1);

>   -     move.b (%a1),%d0
>   -     and.b #253,%d0
>   -     move.b %d0,(%a1)

>   +     bclr.b #1, (%a1)

 
> As you can see, the C optimized code gives three instructions where
> only one would do.  Why is this?  Doesn't gcc know about the bset/bclr
> operations on m68332?  

The m68k backend knows about bset/bclr, but it's not matching on the code
above.  This is because it's not set up for simple and's, which is what the
constant-folded version of your macro looks like.  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.

> If so, how can I fix it?

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. */ 
}")

where "constant_mask_operand" is a predicate you create to make sure the
constant is a bit mask with only one bit clear.  You'll also want to place the
new patterns before things like andqi3, lest the combiner combine the first
'set' with the 'and' before it makes an attempt to combine all three
(set/and/set.)  But again, the above pattern will fail on volatile memory refs
during combine. 

Another possibility is a peephole (less useful because it won't save you the
register, but it will work with volatile pointers.)  Something like:

(define_peephole
  [(set (match_operand:QI 0 "register_operand" "=r")
	(match_operand:QI 1 "memory_operand" "m"))
   (set (match_dup 0) 
    	(and (match_dup 0) (match_operand:QI 2 "constant_mask_operand" "")))
   (set (match_dup 1) (match_dup 0))]
   "dead_or_set_p (NEXT_INSN (NEXT_INSN (ins1)), operands[0])"
   "* 
{
 /* figure out which bit is zero in the constant, and
    return "bclr bit, %0" for that case. */ 
}")

The above patterns aren't tested; they are just ideas.

				-Clint