On Thu, Mar 7, 2024 at 11:29 PM Segher Boessenkool
<seg...@kernel.crashing.org> wrote:
>
> On Thu, Mar 07, 2024 at 11:07:18PM +0100, Uros Bizjak wrote:
> > On Thu, Mar 7, 2024 at 10:37 PM Segher Boessenkool
> > <seg...@kernel.crashing.org> wrote:
> > > > but can be something else, such as the above noted
> > > >
> > > >  (unspec:DI [
> > > >          (reg:CC 17 flags)
> > > >      ] UNSPEC_PUSHFL)
> > >
> > > But that is invalid RTL?  The only valid use of a CC is written as
> > > cc-compared-to-0.  It means "the result of something compared to 0,
> > > stored in that cc reg".
> > >
> > > (And you can copy a CC reg around, but that is not a use ;-) )
> >
> > How can we describe a pushfl then?
>
> (unspec:DI [
>         (compare:CC) ((reg:CC 17 flags) (const_int 0))
>     ] UNSPEC_PUSHFL)
>
> or something like that?
>
> > It was changed to its current form
> > in [1], but I suspect that the code will try to update it even when
> > pushfl is implemented as a direct move from a register (as was defined
> > previously).
> >
> > BTW: Unspecs are used in a couple of other places for other targets [2].
> >
> > [1] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=112494#c5
> > [2] https://gcc.gnu.org/pipermail/gcc-patches/2023-December/639743.html
>
> There is nothing wront with unspecs.  You cannot use a CCmode value
> without comparing it to 0, though.  The exact kind of comparison
> determines what bits are valid (and have what meaning) in your CC reg,
> even!

The pushfl can be considered as a transparent move, separate bits have
no meaning there. I don't see why using unspec should be any different
than using "naked" register (please also see below, current source
code may update "naked" reg as well). What constitutes use is "(cmp:CC
(CC reg) (const_int 0))" around the register and I think that without
this RTX around the CC reg its use should not be updated in any way.

> > > > The source code that deals with the *user* of the CC register assumes
> > > > the former form, so it blindly tries to update the mode of the CC
> > > > register inside LT comparison RTX
> > >
> > > Would you like it better if there was an assert for this?  There are
> > > very many RTL requirements that aren't chacked for currently :-/
> >
> > In this case - yes. Assert signals that something is unsupported (or
> > invalid), way better than silently corrupting some memory, reporting
> > the corruption only with checking enabled.
>
> Yeah.  The way RTL checking works makes this hard to do in most cases.
> Hrm.  (It cannot easily look at context, only inside of the current RTX).
>
> > > The unspec should have the CC compared with 0 as argument.
> >
> > But this does not do what pushfl does... It pushes the register to the 
> > stack.
>
> Can't you just describe the dataflow then, without an unspec?  An unspec
> by definition does some (unspecified) operation on the data.

Previously, it was defined as:

 (define_insn "*pushfl<mode>2"
   [(set (match_operand:W 0 "push_operand" "=<")
 (match_operand:W 1 "flags_reg_operand"))]

But Wmode, AKA SI/DImode is not CCmode. And as said in my last
message, nothing prevents current source code to try to update the CC
register here.

Uros.

Reply via email to