Re: Modifying ARM code generator for elimination of 8bit writes - need help

[Wolfgang Mües <wolfgang at iksw-muees dot de>]

Hello,

after getting a "working" version of the gcc 4.0.2 with the Nintendo 
8-bit-write problem, I was busy the last weeks trying to adapt the 
linux system (replacing I/O with writeb() macros, removing strb 
assembler calls).

However, it turned out that the sources of the linux kernel are a far 
more demanding test than every single small test case.

I have tried my very best to implement the last patch from Rask (thank 
you very much!). There was one place I was not shure I have coded the
right solution:

Rasks patch (gcc 4.2.x):

> +;; Match register operands or memory operands of the form (mem (reg
> ...)), +;; as permitted by the "Q" memory constraint.
> +(define_predicate "reg_or_Qmem_operand"
> +  (ior (match_operand 0 "register_operand")
> +       (and (match_code "mem")
> +	    (match_code "reg" "0")))
> +)
> +

My patch (without the second operand for match_code):

> ;; Match register operands or memory operands of the form (mem (reg
> ...)), ;; as permitted by the "Q" memory constraint.
> (define_predicate "reg_or_Qmem_operand"
>   (ior (match_operand 0 "register_operand")
>        (and (match_code "mem")
> 	    (match_test "GET_CODE (XEXP (op, 0)) == REG")))
> )

Is this the right substitution?

If I compile the linux kernel with this patch, many files get compiled 
without problems, but in fs/vfat/namei.c I get:

> fs/vfat/namei.c: In function 'vfat_add_entry':
> fs/vfat/namei.c:694: error: unrecognizable insn:
> (insn 2339 2338 2340 188 (set (mem/s/j:QI (reg:SI 14 lr) [0
> <variable>.attr+0 S1 A8]) (reg:QI 12 ip)) -1 (nil)
>     (nil))
> fs/vfat/namei.c:694: internal compiler error: in extract_insn, at
> recog.c:2020 Please submit a full bug report,

I can't see what is going on here...

regards

Wolfgang


The full patch of Rask is appended below:

> Index: gcc/config/arm/arm.h
> ===================================================================
> --- gcc/config/arm/arm.h	(revision 114119)
> +++ gcc/config/arm/arm.h	(working copy)
> @@ -1094,6 +1094,8 @@
>     ? vfp_secondary_reload_class (MODE, X)			\
>
>     : TARGET_ARM							\
>
>     ? (((MODE) == HImode && ! arm_arch4 && true_regnum (X) == -1) \
> +   || ((MODE) == QImode && TARGET_ARM && TARGET_SWP_BYTE_WRITES	\
> +       && true_regnum (X) == -1)				\
>      ? GENERAL_REGS : NO_REGS)					\
>
>     : THUMB_SECONDARY_OUTPUT_RELOAD_CLASS (CLASS, MODE, X))
>
> Index: gcc/config/arm/arm.opt
> ===================================================================
> --- gcc/config/arm/arm.opt	(revision 114119)
> +++ gcc/config/arm/arm.opt	(working copy)
> @@ -153,3 +153,7 @@
>  mwords-little-endian
>  Target Report RejectNegative Mask(LITTLE_WORDS)
>  Assume big endian bytes, little endian words
> +
> +mswp-byte-writes
> +Target Report Mask(SWP_BYTE_WRITES)
> +Use the swp instruction for byte writes. The default is to use str
> Index: gcc/config/arm/predicates.md
> ===================================================================
> --- gcc/config/arm/predicates.md	(revision 114119)
> +++ gcc/config/arm/predicates.md	(working copy)
> @@ -125,6 +125,14 @@
>
>  			 || (GET_CODE (op) == REG
>
>  			     && REGNO (op) >= FIRST_PSEUDO_REGISTER)))")))
>
> +;; Match register operands or memory operands of the form (mem (reg
> ...)), +;; as permitted by the "Q" memory constraint.
> +(define_predicate "reg_or_Qmem_operand"
> +  (ior (match_operand 0 "register_operand")
> +       (and (match_code "mem")
> +	    (match_code "reg" "0")))
> +)
> +
>  ;; True for valid operands for the rhs of an floating point insns.
>  ;;   Allows regs or certain consts on FPA, just regs for everything
> else. (define_predicate "arm_float_rhs_operand"
> Index: gcc/config/arm/arm.md
> ===================================================================
> --- gcc/config/arm/arm.md	(revision 114119)
> +++ gcc/config/arm/arm.md	(working copy)
> @@ -5151,6 +5151,16 @@
>        emit_insn (gen_movsi (operands[0], operands[1]));
>        DONE;
>      }
> +  if (TARGET_ARM && TARGET_SWP_BYTE_WRITES)
> +    {
> +      /* Ensure that operands[0] is (mem (reg ...)) if a memory
> operand. */ +      if (MEM_P (operands[0]) && !REG_P (XEXP
> (operands[0], 0))) +	    operands[0]
> +	      = replace_equiv_address (operands[0],
> +				       copy_to_reg (XEXP (operands[0], 0)));
> +      emit_insn (gen__arm_movqi_insn_swp (operands[0],
> operands[1])); +      DONE;
> +    }
>    "
>  )
>
> @@ -5158,7 +5168,7 @@
>  (define_insn "*arm_movqi_insn"
>    [(set (match_operand:QI 0 "nonimmediate_operand" "=r,r,r,m")
>  	(match_operand:QI 1 "general_operand" "rI,K,m,r"))]
> -  "TARGET_ARM
> +  "TARGET_ARM && !TARGET_SWP_BYTE_WRITES
>     && (   register_operand (operands[0], QImode)
>
>         || register_operand (operands[1], QImode))"
>
>    "@
> @@ -5170,6 +5180,31 @@
>     (set_attr "predicable" "yes")]
>  )
>
> +;; This is primarily a hack for the Nintendo DS external RAM.
> +(define_insn "_arm_movqi_insn_swp"
> +  [(set (match_operand:QI 0 "reg_or_Qmem_operand" "=r,r,r,Q")
> +	(match_operand:QI 1 "general_operand" "rI,K,m,r"))
> +        (clobber (match_scratch:QI 2 "=X,X,X,r"))]
> +  "TARGET_ARM && TARGET_SWP_BYTE_WRITES
> +   && (   register_operand (operands[0], QImode)
> +       || register_operand (operands[1], QImode))"
> +  "@
> +   mov%?\\t%0, %1
> +   mvn%?\\t%0, #%B1
> +   ldr%?b\\t%0, %1
> +   swp%?b\\t%2, %1, [%|%m0]"
> +  [(set_attr "type" "*,*,load1,store1")
> +   (set_attr "predicable" "yes")]
> +)
> +
> +;; The earlyclobber is required by default_secondary_reload() in
> targhooks.c. +(define_expand "reload_outqi"
> +  [(set (match_operand:QI 0 "memory_operand" "=Q")
> +	(match_operand:QI 1 "register_operand" "r"))
> +   (clobber (match_operand:QI 2 "register_operand" "=&r"))]
> +  "TARGET_ARM && TARGET_SWP_BYTE_WRITES"
> +)
> +
>  (define_insn "*thumb_movqi_insn"
>    [(set (match_operand:QI 0 "nonimmediate_operand" "=l,l,m,*r,*h,l")
>  	(match_operand:QI 1 "general_operand"      "l, m,l,*h,*r,I"))]

-- 
We're back to the times when men were men 
and wrote their own device drivers.

(Linus Torvalds)