Chung-Lin Tang <clt...@codesourcery.com> 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