https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63783

--- Comment #6 from Oleg Endo <olegendo at gcc dot gnu.org> ---
(In reply to Michael Karcher from comment #5)
> (In reply to Oleg Endo from comment #4)
> > I'm not sure about this.  The first hunk of your patch that removes the
> > example in the top comment block should be valid, as far as I can see at the
> > moment.  This is because the result of the bitwise not is then again tested
> > for zero before the conditional jump.

... the above is true for bitwise not values of zero only...

> As far as I understand the SH4 architecture, "TST r0,r0" sets the T bit if
> r0 is zero, and clears it otherwise. So
>   MOV   r1,r0        ; store r1 to r0, for exposition only
>   TST   r0,r0        ; T bit set if r1 == 0, cleared if r1 != 0
>   MOVT  r0           ; r0 set to 1 if r1 == 0, cleared if r1 != 0
>   TST   r0,r0        ; T bit set if r1 != 0, cleared if r1 == 0
> while on the other hand
>   NOT   r1,r0        ; bitwise negate r1 to r0 (so r0 gets 0 if r1 == -1)
>   TST   r0,r0        ; T bit set if r1 == -1, cleared if r1 != -1
> 
> The first block clear T only if r1 equals zero, the second block clears T
> for all values of r1 except -1. So they are only equivalent if you can prove
> that r1 is either zero or -1, which you typically can't, especially as using
> 1 for true is typical C semantic. Because it seems that it is rare you can
> successfully prove that r1 either 0 or -1, I decided to remove that
> transformation alltogether. And to keep the comment accurate, I had to
> remove it from there, too.

... and because of this, you're right that the bitwise not can't be used for
this kind of thing.  That was my thinko when writing the pass.  If the input
value for the bitwise not is something != -1 then it will not output zero
obviously and this is wrong.

> For the transformation to be valid, you would need a logical not instruction
> instead of the bitwise not instruction that sets the desination register to
> zero if the source register contains any non-zero value, not just if the
> source register contains -1.

There is a 2 insn sequence to do that on SH:
   tst  rm,rm
   movt rn

As far as I can see it, emitting this sequence instead of the bitwise not insn
would fix the problem.  This can be done by changing the function
sh_treg_combine::make_not_reg_insn.  It should be safe to emit that sequence
(which clobbers the T reg) instead of the bitwise not.

> Possibly your confusion is created by "NOTT" (if it were available on SH4)
> being in fact valid in that example to negate the condition. This is due to
> the fact, that for 1-bit values (only!) bitwise negation and boolean
> negation are the same.

No, this confusion is not related to the SH2A nott insn.  I was just thinking
of input values of zero for the bitwise not.

> Looking at the generated assembly code, I verified that indeed the pattern
> above is used, so "condition" gets stored in the "T" bit, and a conditional
> branch (BF) is used to skip the incrementation if it is false, i.e. cleared.
> After applying the transformation shown above, the value 1 in
> decision_result gets negated to -2 (0xFFFFFFFE), which is non-zero, so
> truecount gets increased if even though decision_result should be false
> after boolean negation.

OK, thanks for the confirmation.  I had a closer look at the current code and
your patch.  I think that the bitwise-not code paths should not be removed
entirely, as they handle the "mixed normal and inverting cstores" cases.  I
will prepare a patch in a couple of days.

Reply via email to