Chung-Lin Tang <[email protected]> writes:
>>> +;; Integer logical Operations
>>> +
>>> +(define_code_iterator LOGICAL [and ior xor])
>>> +(define_code_attr logical_asm [(and "and") (ior "or") (xor "xor")])
>>> +
>>> +(define_insn "<code>si3"
>>> + [(set (match_operand:SI 0 "register_operand" "=r,r,r")
>>> + (LOGICAL:SI (match_operand:SI 1 "register_operand" "%r,r,r")
>>> + (match_operand:SI 2 "logical_operand" "rM,J,K")))]
>>> + ""
>>> + "@
>>> + <logical_asm>\\t%0, %1, %z2
>>> + <logical_asm>%i2\\t%0, %1, %2
>>> + <logical_asm>h%i2\\t%0, %1, %U2"
>>> + [(set_attr "type" "alu")])
>>> +
>>> +(define_insn "*norsi3"
>>> + [(set (match_operand:SI 0 "register_operand" "=r")
>>> + (and:SI (not:SI (match_operand:SI 1 "register_operand" "%r"))
>>> + (not:SI (match_operand:SI 2 "reg_or_0_operand" "rM"))))]
>>> + ""
>>> + "nor\\t%0, %1, %z2"
>>> + [(set_attr "type" "alu")])
>>
>> M constraints (for const0_rtx) and reg_or_0 seem unnecessary, no such
>> RTL should make it to this point.
>
> Such RTL does appear under -O0. Removing the 'M' will also
> require a bit of re-working the operand printing mechanics; not a lot of
> work, but I'd rather keep it as is. The behavior of using the zero
> register for a 0-value is also more expected in nios2, I think.
Hmm, but if we get (not (const_int 0)) then that sounds like a bug,
since (and (not X) (not (const_int 0))) should reduce to (not X).
IMO target-independent code shouldn't try to create the nor-with-0
form and backends shouldn't match it.
Why would removing 'M' affect the printing mechanism? Naively I'd
have expected:
(define_insn "*norsi3"
[(set (match_operand:SI 0 "register_operand" "=r")
(and:SI (not:SI (match_operand:SI 1 "register_operand" "r"))
(not:SI (match_operand:SI 2 "register_operand" "r"))))]
""
"nor\\t%0, %1, %2"
[(set_attr "type" "alu")])
to just work.
>>> +;; Integer comparisons
>>> +
>>> +(define_code_iterator EQNE [eq ne])
>>> +(define_insn "nios2_cmp<code>"
>>> + [(set (match_operand:SI 0 "register_operand" "=r")
>>> + (EQNE:SI (match_operand:SI 1 "reg_or_0_operand" "%rM")
>>> + (match_operand:SI 2 "arith_operand" "rI")))]
>>> + ""
>>> + "cmp<code>%i2\\t%0, %z1, %z2"
>>> + [(set_attr "type" "alu")])
>>
>> Once again, using reg_or_0 and "M" seems pointless.
>
> The compares don't support all operations, with only the second operand
> capable of an immediate. Using 'rM' should theoretically allow more
> commutative swapping.
But rtl-wise, we should never generate an EQ or NE with two constants.
And if one operand is constant then it's supposed to be the second.
The "%" should give commutativity on its own, without the "M".
>>> + emit_insn
>>> + (gen_rtx_SET (Pmode, tmp,
>>> + gen_int_mode (cfun->machine->save_regs_offset,
>>> + Pmode)));
>>
>> Shouldn't have a mode on the SET, but really should just call
>> emit_move_insn. Similar cases elsewhere, search for "gen_rtx_SET (Pmode".
>
> I've removed the modes on SET, though I prefer the more bare generation
> of the insns in some contexts; emit_move_insn() seems to have a lot
> under the hood.
There shouldn't be anything to be afraid of though. Target-independent
code would use emit_move_insn for this though, so it needs to just work.
>>> + HOST_WIDE_INT var_size; /* # of var. bytes allocated. */
>>
>> Not the first time they occur in this file, but I suppose I should
>> mention that these in-line comments are probably just outside our coding
>> guidelines.
>
> Deleted the comments outside the struct defs.
FWIW, I think it was more that comments should be above the field rather
than tagged on the right. (One of the big problems with right-column comments
is that people tend to make them too short.)
Thanks,
Richard
