public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH 2/9] Libgcc bits and the back end itself
@ 2017-04-01 16:48 Andrew Jenner
  2017-04-01 22:25 ` Segher Boessenkool
  2017-04-05 17:40 ` Joseph Myers
  0 siblings, 2 replies; 3+ messages in thread
From: Andrew Jenner @ 2017-04-01 16:48 UTC (permalink / raw)
  To: GCC Patches

[-- Attachment #1: Type: text/plain, Size: 724 bytes --]

This part is the back end itself.

libgcc/
2017-04-01  Andrew Jenner  <andrew@codesourcery.com>
	Rask Ingemann Lambertsen  <rask@sygehus.dk>

	* config.host: Add support for ia16.
	* ia16/t-ia16: New.

gcc/
2017-04-01  Andrew Jenner  <andrew@codesourcery.com>
	Rask Ingemann Lambertsen  <rask@sygehus.dk>

	* config.gcc: Add support for ia16.
	* config/ia16/ia16.md: New.
	* config/ia16/ia16-peepholes.md: New.
	* config/ia16/ia16.opt: New.
	* config/ia16/ia16.c: New.
	* config/ia16/t-ia16: New.
	* config/ia16/ia16-modes.def: New.
	* config/ia16/predicates.md: New.
	* config/ia16/constraints.md: New.
	* config/ia16/ia16.h: New.
	* config/ia16/ia16-protos.h: New.
	* config/ia16/elf.h: New.
	* config/ia16/elks.h: New.



[-- Attachment #2: gcc_ia16.patch.gz --]
[-- Type: application/gzip, Size: 52980 bytes --]

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH 2/9] Libgcc bits and the back end itself
  2017-04-01 16:48 [PATCH 2/9] Libgcc bits and the back end itself Andrew Jenner
@ 2017-04-01 22:25 ` Segher Boessenkool
  2017-04-05 17:40 ` Joseph Myers
  1 sibling, 0 replies; 3+ messages in thread
From: Segher Boessenkool @ 2017-04-01 22:25 UTC (permalink / raw)
  To: Andrew Jenner; +Cc: GCC Patches

Hi!

Looks pretty good.  Some of it shows its age.  Indents are weird in many
places (tabs that should be two spaces; tabs after spaces; eight spaces
instead of a tab; continuation lines aligned wrong).  A few more specific
comments (many of those happen more than once):

On Sat, Apr 01, 2017 at 05:47:55PM +0100, Andrew Jenner wrote:
+; Complex modes excpet CQI, which needs special treatment.

Typo ("except").

+(define_code_attr  cond_shift
+       [(ashift "1") (lshiftrt "1") (rotate "1") (rotatert "1")
+        (ashiftrt "!CONST_INT_P (operands[2]) || INTVAL (operands[2]) < GET_MODE_BITSIZE (GET_MODE (operands[0])) - 1")])

That line is a bit long.

+  do {
+    i -= 2;
+    tmp = simplify_gen_subreg (HImode, operands[0], <MODE>mode, i);
+    emit_insn (gen_pushhi1 (tmp));
+  } while (i > 0);

The { } should be on separate lines, and with an extra indent.

+  switch (GET_CODE (zext))
+    {
+      case ZERO_EXTEND:

The case should be indented one less (the same as the {).

+; Combine sometimes doesn't try to split off an outer ZERO_EXTEND, perhaps
+; because the combined insn was made up of only two insns.  IMHO, this sucks.

Yes, combine doesn't yet do 2->2 combinations.  And yes it sucks.

+  /* For the benefit of the lower-subreg pass, create a HImode subreg if the
+   * zero_extract operand is larger than HImode.  */

s/\*/ / on that last line.

+  "INTVAL (operands[2]) % BITS_PER_UNIT == 0
+   && (!MEM_P (operands[0]) || !MEM_P (operands[1]))"
+  "#"
+  "reload_completed"

This should probably be "&& reload_completed".

+(define_insn "*xchg<mode>2"
+  [(set (match_operand:LE16 0 "register_operand" "+<r>")
+       (match_operand:LE16 1 "nonimmediate_operand" "+<r>m"))
+   (set (match_dup 1) (match_dup 0))]
+  "!MEM_P (operands[0]) || !MEM_P (operands[1])"

operands[0] cannot be a MEM anyway?

+; Combine creates such insns. Get rid of the clobber and the scratch register.
+(define_split
+  [(set (reg:SET_SCZ CC_REG)
+       (compare:SET_SCZ (plus:MO (match_operand:MO 0 "nonimmediate_operand")
+                                 (match_operand:MO 1 "const_int_operand"))
+                       (const_int 0)))
+   (clobber (scratch:MO))]
+  "INTVAL (operands[1]) != -32768"
+  [(set (reg:CC CC_REG)        (compare:CC (match_dup 0) (match_dup 2)))]
+{
+  operands[2] = GEN_INT (-INTVAL (operands[1]));
+})

Hrm, if you have a testcase, could you create a PR please?

Do you need to special case -32768?  You could use gen_int_mode instead.

+; Combine prefers ZERO_EXTEND over AND.  Undo this transformation.
+(define_insn_and_split "*zero_extendqihi2"

Combine now does tries the AND form automatically if the ZERO_EXTEND form
does *not* match.


Anyway, looks pretty good :-)


Segher

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH 2/9] Libgcc bits and the back end itself
  2017-04-01 16:48 [PATCH 2/9] Libgcc bits and the back end itself Andrew Jenner
  2017-04-01 22:25 ` Segher Boessenkool
@ 2017-04-05 17:40 ` Joseph Myers
  1 sibling, 0 replies; 3+ messages in thread
From: Joseph Myers @ 2017-04-05 17:40 UTC (permalink / raw)
  To: Andrew Jenner; +Cc: GCC Patches

Copyright notices should now use a single range of years (e.g. 2005-2017).

-- 
Joseph S. Myers
joseph@codesourcery.com

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2017-04-05 17:40 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-04-01 16:48 [PATCH 2/9] Libgcc bits and the back end itself Andrew Jenner
2017-04-01 22:25 ` Segher Boessenkool
2017-04-05 17:40 ` Joseph Myers

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).