[00/11] Diagnose ambiguous .md attribute uses

[00/11] Diagnose ambiguous .md attribute uses

[Richard Sandiford]

While doing the SVE ACLE work, I hit a case in which a pattern had
two iterators and used an attribute without saying which iterator
it was referring to.  The choice of iterator made a difference and
in this case the implicit choice happened to be the "wrong" one.

This series raises an error for cases in which an unprefixes attribute
could expand to different things depending on the iterator chosen.
It also tries to fix all instances in the ports.

Bootstrapped & regression-tested on aarch64-linux-gnu and
x86_64-linux-gnu.  Also tested by building at least one target
per CPU directory and (a) checking for errors and (b) diffing the
autogenerated files before and after the patch.

Richard

[01/11] [arch64] Fix ambiguous .md attribute uses

[Richard Sandiford]

This patch is part of a series that fixes ambiguous attribute
uses in .md files, i.e. cases in which attributes didn't use
<ITER:ATTR> to specify an iterator, and in which <ATTR> could
have different values depending on the iterator chosen.

No behavioural change except for dropping the unused SVE
divide permutations.

I can self-approve the SVE bits, but OK for the rest?

Richard


2019-07-05  Richard Sandiford  <richard.sandiford@arm.com>

gcc/
	* config/aarch64/aarch64.md (*compare_condjump<mode>)
	(loadwb_pair<GPI:mode>_<P:mode>, loadwb_pair<GPF:mode>_<P:mode>)
	(storewb_pair<GPI:mode>_<P:mode>, storewb_pair<GPF:mode>_<P:mode>)
	(*ands<mode>_compare0): Fix ambiguous uses of .md attributes.
	* config/aarch64/aarch64-simd.md
	(*aarch64_get_lane_extend<GPI:mode><VDQQH:mode>): Likewise.
	(*aarch64_get_lane_zero_extend<GPI:mode><VDQQH:mode>): Likewise.
	* config/aarch64/aarch64-sve.md
	(while_ult<GPI:mode><PRED_ALL:mode>): Likewise.
	(*cond_<optab><mode>_any): Fix SVE_I/SVE_SDI typo.

Index: gcc/config/aarch64/aarch64.md
===================================================================
--- gcc/config/aarch64/aarch64.md	2019-07-03 20:51:55.225747145 +0100
+++ gcc/config/aarch64/aarch64.md	2019-07-05 15:04:46.680794809 +0100
@@ -567,14 +567,14 @@ (define_insn "condjump"
 ;; 	sub	x0, x1, #(CST & 0xfff000)
 ;; 	subs	x0, x0, #(CST & 0x000fff)
 ;; 	b<ne,eq> .Label
-(define_insn_and_split "*compare_condjump<mode>"
+(define_insn_and_split "*compare_condjump<GPI:mode>"
   [(set (pc) (if_then_else (EQL
 			      (match_operand:GPI 0 "register_operand" "r")
 			      (match_operand:GPI 1 "aarch64_imm24" "n"))
 			   (label_ref:P (match_operand 2 "" ""))
 			   (pc)))]
-  "!aarch64_move_imm (INTVAL (operands[1]), <MODE>mode)
-   && !aarch64_plus_operand (operands[1], <MODE>mode)
+  "!aarch64_move_imm (INTVAL (operands[1]), <GPI:MODE>mode)
+   && !aarch64_plus_operand (operands[1], <GPI:MODE>mode)
    && !reload_completed"
   "#"
   "&& true"
@@ -582,11 +582,12 @@ (define_insn_and_split "*compare_condjum
   {
     HOST_WIDE_INT lo_imm = UINTVAL (operands[1]) & 0xfff;
     HOST_WIDE_INT hi_imm = UINTVAL (operands[1]) & 0xfff000;
-    rtx tmp = gen_reg_rtx (<MODE>mode);
-    emit_insn (gen_add<mode>3 (tmp, operands[0], GEN_INT (-hi_imm)));
-    emit_insn (gen_add<mode>3_compare0 (tmp, tmp, GEN_INT (-lo_imm)));
+    rtx tmp = gen_reg_rtx (<GPI:MODE>mode);
+    emit_insn (gen_add<GPI:mode>3 (tmp, operands[0], GEN_INT (-hi_imm)));
+    emit_insn (gen_add<GPI:mode>3_compare0 (tmp, tmp, GEN_INT (-lo_imm)));
     rtx cc_reg = gen_rtx_REG (CC_NZmode, CC_REGNUM);
-    rtx cmp_rtx = gen_rtx_fmt_ee (<EQL:CMP>, <MODE>mode, cc_reg, const0_rtx);
+    rtx cmp_rtx = gen_rtx_fmt_ee (<EQL:CMP>, <GPI:MODE>mode,
+				  cc_reg, const0_rtx);
     emit_jump_insn (gen_condjump (cmp_rtx, cc_reg, operands[2]));
     DONE;
   }
@@ -1505,8 +1506,8 @@ (define_insn "loadwb_pair<GPI:mode>_<P:m
           (mem:GPI (plus:P (match_dup 1)
                    (match_operand:P 5 "const_int_operand" "n"))))])]
   "INTVAL (operands[5]) == GET_MODE_SIZE (<GPI:MODE>mode)"
-  "ldp\\t%<w>2, %<w>3, [%1], %4"
-  [(set_attr "type" "load_<ldpstp_sz>")]
+  "ldp\\t%<GPI:w>2, %<GPI:w>3, [%1], %4"
+  [(set_attr "type" "load_<GPI:ldpstp_sz>")]
 )
 
 (define_insn "loadwb_pair<GPF:mode>_<P:mode>"
@@ -1520,7 +1521,7 @@ (define_insn "loadwb_pair<GPF:mode>_<P:m
           (mem:GPF (plus:P (match_dup 1)
                    (match_operand:P 5 "const_int_operand" "n"))))])]
   "INTVAL (operands[5]) == GET_MODE_SIZE (<GPF:MODE>mode)"
-  "ldp\\t%<w>2, %<w>3, [%1], %4"
+  "ldp\\t%<GPF:w>2, %<GPF:w>3, [%1], %4"
   [(set_attr "type" "neon_load1_2reg")]
 )
 
@@ -1553,8 +1554,8 @@ (define_insn "storewb_pair<GPI:mode>_<P:
                    (match_operand:P 5 "const_int_operand" "n")))
           (match_operand:GPI 3 "register_operand" "r"))])]
   "INTVAL (operands[5]) == INTVAL (operands[4]) + GET_MODE_SIZE (<GPI:MODE>mode)"
-  "stp\\t%<w>2, %<w>3, [%0, %4]!"
-  [(set_attr "type" "store_<ldpstp_sz>")]
+  "stp\\t%<GPI:w>2, %<GPI:w>3, [%0, %4]!"
+  [(set_attr "type" "store_<GPI:ldpstp_sz>")]
 )
 
 (define_insn "storewb_pair<GPF:mode>_<P:mode>"
@@ -1569,7 +1570,7 @@ (define_insn "storewb_pair<GPF:mode>_<P:
                    (match_operand:P 5 "const_int_operand" "n")))
           (match_operand:GPF 3 "register_operand" "w"))])]
   "INTVAL (operands[5]) == INTVAL (operands[4]) + GET_MODE_SIZE (<GPF:MODE>mode)"
-  "stp\\t%<w>2, %<w>3, [%0, %4]!"
+  "stp\\t%<GPF:w>2, %<GPF:w>3, [%0, %4]!"
   [(set_attr "type" "neon_store1_2reg<q>")]
 )
 
@@ -4782,7 +4783,7 @@ (define_insn "*and<mode>_compare0"
   [(set_attr "type" "alus_imm")]
 )
 
-(define_insn "*ands<mode>_compare0"
+(define_insn "*ands<GPI:mode>_compare0"
   [(set (reg:CC_NZ CC_REGNUM)
 	(compare:CC_NZ
 	 (zero_extend:GPI (match_operand:SHORT 1 "register_operand" "r"))
Index: gcc/config/aarch64/aarch64-simd.md
===================================================================
--- gcc/config/aarch64/aarch64-simd.md	2019-07-03 20:51:55.225747145 +0100
+++ gcc/config/aarch64/aarch64-simd.md	2019-07-05 15:04:46.676794843 +0100
@@ -3135,30 +3135,31 @@ (define_expand "vcondu<mode><v_cmp_mixed
 (define_insn "*aarch64_get_lane_extend<GPI:mode><VDQQH:mode>"
   [(set (match_operand:GPI 0 "register_operand" "=r")
 	(sign_extend:GPI
-	  (vec_select:<VEL>
+	  (vec_select:<VDQQH:VEL>
 	    (match_operand:VDQQH 1 "register_operand" "w")
 	    (parallel [(match_operand:SI 2 "immediate_operand" "i")]))))]
   "TARGET_SIMD"
   {
-    operands[2] = aarch64_endian_lane_rtx (<MODE>mode, INTVAL (operands[2]));
+    operands[2] = aarch64_endian_lane_rtx (<VDQQH:MODE>mode,
+					   INTVAL (operands[2]));
     return "smov\\t%<GPI:w>0, %1.<VDQQH:Vetype>[%2]";
   }
-  [(set_attr "type" "neon_to_gp<q>")]
-)
-
-(define_insn "*aarch64_get_lane_zero_extend<GPI:mode><VDQQH:mode>"
-  [(set (match_operand:GPI 0 "register_operand" "=r")
-	(zero_extend:GPI
-	  (vec_select:<VEL>
-	    (match_operand:VDQQH 1 "register_operand" "w")
-	    (parallel [(match_operand:SI 2 "immediate_operand" "i")]))))]
-  "TARGET_SIMD"
-  {
-    operands[2] = aarch64_endian_lane_rtx (<VDQQH:MODE>mode,
-					   INTVAL (operands[2]));
-    return "umov\\t%w0, %1.<Vetype>[%2]";
-  }
-  [(set_attr "type" "neon_to_gp<q>")]
+  [(set_attr "type" "neon_to_gp<VDQQH:q>")]
+)
+
+(define_insn "*aarch64_get_lane_zero_extend<GPI:mode><VDQQH:mode>"
+  [(set (match_operand:GPI 0 "register_operand" "=r")
+	(zero_extend:GPI
+	  (vec_select:<VDQQH:VEL>
+	    (match_operand:VDQQH 1 "register_operand" "w")
+	    (parallel [(match_operand:SI 2 "immediate_operand" "i")]))))]
+  "TARGET_SIMD"
+  {
+    operands[2] = aarch64_endian_lane_rtx (<VDQQH:MODE>mode,
+					   INTVAL (operands[2]));
+    return "umov\\t%w0, %1.<VDQQH:Vetype>[%2]";
+  }
+  [(set_attr "type" "neon_to_gp<VDQQH:q>")]
 )
 
 ;; Lane extraction of a value, neither sign nor zero extension
Index: gcc/config/aarch64/aarch64-sve.md
===================================================================
--- gcc/config/aarch64/aarch64-sve.md	2019-07-03 20:51:55.225747145 +0100
+++ gcc/config/aarch64/aarch64-sve.md	2019-07-05 15:04:46.680794809 +0100
@@ -1363,7 +1363,7 @@ (define_insn_and_rewrite "*while_ult<GPI
   ;; don't have an unnecessary PTRUE.
   "&& !CONSTANT_P (operands[1])"
   {
-    operands[1] = CONSTM1_RTX (<MODE>mode);
+    operands[1] = CONSTM1_RTX (<PRED_ALL:MODE>mode);
   }
 )
 

[02/11] [arm] Fix ambiguous .md attribute uses

[Richard Sandiford]

This patch is part of a series that fixes ambiguous attribute
uses in .md files, i.e. cases in which attributes didn't use
<ITER:ATTR> to specify an iterator, and in which <ATTR> could
have different values depending on the iterator chosen.

I think this is a genuine bugfix for Thumb-1, since previously the
LDREX width was taken from the SImode success result rather than the
memory mode:

-#define HAVE_atomic_compare_and_swapt1qi_1 ((TARGET_HAVE_LDREX && TARGET_HAVE_MEMORY_BARRIER) && (TARGET_THUMB1))
-#define HAVE_atomic_compare_and_swapt1hi_1 ((TARGET_HAVE_LDREX && TARGET_HAVE_MEMORY_BARRIER) && (TARGET_THUMB1))
-#define HAVE_atomic_compare_and_swapt1di_1 ((TARGET_HAVE_LDREX && TARGET_HAVE_MEMORY_BARRIER) && (TARGET_THUMB1))
+#define HAVE_atomic_compare_and_swapt1qi_1 ((TARGET_HAVE_LDREXBH && TARGET_HAVE_MEMORY_BARRIER) && (TARGET_THUMB1))
+#define HAVE_atomic_compare_and_swapt1hi_1 ((TARGET_HAVE_LDREXBH && TARGET_HAVE_MEMORY_BARRIER) && (TARGET_THUMB1))
+#define HAVE_atomic_compare_and_swapt1di_1 ((TARGET_HAVE_LDREXD && ARM_DOUBLEWORD_ALIGN \
+       && TARGET_HAVE_MEMORY_BARRIER) && (TARGET_THUMB1))

The same goes for the predicate and constraints in
@atomic_compare_and_swapt1di_1, which previously used the
SI values from the success result.


2019-07-05  Richard Sandiford  <richard.sandiford@arm.com>

gcc/
	* config/arm/sync.md
	(@atomic_compare_and_swap<CCSI:arch><NARROW:mode>_1): Use
	<NARROW:sync_predtab> instead of (implicitly) <CCSI:sync_predtab>.
	(@atomic_compare_and_swap<CCSI:arch><SIDI:mode>_1): Likewise
	<SIDI:sync_predtab>.  Use <SIDI:cas_cmp_operand> and
	<SIDI:cas_cmp_str>.

Index: gcc/config/arm/sync.md
===================================================================
--- gcc/config/arm/sync.md	2019-07-01 09:37:07.224524452 +0100
+++ gcc/config/arm/sync.md	2019-07-05 15:05:09.088609956 +0100
@@ -201,7 +201,7 @@ (define_insn_and_split "@atomic_compare_
 	   (match_operand:SI 7 "const_int_operand")]		;; mod_f
 	  VUNSPEC_ATOMIC_CAS))
    (clobber (match_scratch:SI 8 "=&r,X,X,X"))]
-  "<sync_predtab>"
+  "<NARROW:sync_predtab>"
   "#"
   "&& reload_completed"
   [(const_int 0)]
@@ -225,14 +225,14 @@ (define_insn_and_split "@atomic_compare_
 	(match_operand:SIDI 2 "mem_noofs_operand" "+Ua,Ua,Ua,Ua"))	;; memory
    (set (match_dup 2)
 	(unspec_volatile:SIDI
-	  [(match_operand:SIDI 3 "<cas_cmp_operand>" "<cas_cmp_str>,lIL*h,J,*r") ;; expect
+	  [(match_operand:SIDI 3 "<SIDI:cas_cmp_operand>" "<SIDI:cas_cmp_str>,lIL*h,J,*r") ;; expect
 	   (match_operand:SIDI 4 "s_register_operand" "r,r,r,r")	;; desired
 	   (match_operand:SI 5 "const_int_operand")		;; is_weak
 	   (match_operand:SI 6 "const_int_operand")		;; mod_s
 	   (match_operand:SI 7 "const_int_operand")]		;; mod_f
 	  VUNSPEC_ATOMIC_CAS))
    (clobber (match_scratch:SI 8 "=&r,X,X,X"))]
-  "<sync_predtab>"
+  "<SIDI:sync_predtab>"
   "#"
   "&& reload_completed"
   [(const_int 0)]

Re: [02/11] [arm] Fix ambiguous .md attribute uses

[Kyrill Tkachov]

Hi Richard,

On 7/5/19 4:10 PM, Richard Sandiford wrote:
> This patch is part of a series that fixes ambiguous attribute
> uses in .md files, i.e. cases in which attributes didn't use
> <ITER:ATTR> to specify an iterator, and in which <ATTR> could
> have different values depending on the iterator chosen.
>
> I think this is a genuine bugfix for Thumb-1, since previously the
> LDREX width was taken from the SImode success result rather than the
> memory mode:
>
> -#define HAVE_atomic_compare_and_swapt1qi_1 ((TARGET_HAVE_LDREX && 
> TARGET_HAVE_MEMORY_BARRIER) && (TARGET_THUMB1))
> -#define HAVE_atomic_compare_and_swapt1hi_1 ((TARGET_HAVE_LDREX && 
> TARGET_HAVE_MEMORY_BARRIER) && (TARGET_THUMB1))
> -#define HAVE_atomic_compare_and_swapt1di_1 ((TARGET_HAVE_LDREX && 
> TARGET_HAVE_MEMORY_BARRIER) && (TARGET_THUMB1))
> +#define HAVE_atomic_compare_and_swapt1qi_1 ((TARGET_HAVE_LDREXBH && 
> TARGET_HAVE_MEMORY_BARRIER) && (TARGET_THUMB1))
> +#define HAVE_atomic_compare_and_swapt1hi_1 ((TARGET_HAVE_LDREXBH && 
> TARGET_HAVE_MEMORY_BARRIER) && (TARGET_THUMB1))
> +#define HAVE_atomic_compare_and_swapt1di_1 ((TARGET_HAVE_LDREXD && 
> ARM_DOUBLEWORD_ALIGN \
> +       && TARGET_HAVE_MEMORY_BARRIER) && (TARGET_THUMB1))
>
> The same goes for the predicate and constraints in
> @atomic_compare_and_swapt1di_1, which previously used the
> SI values from the success result.
>
>
Ok.

Thanks,

Kyrill

> 2019-07-05  Richard Sandiford <richard.sandiford@arm.com>
>
> gcc/
>         * config/arm/sync.md
> (@atomic_compare_and_swap<CCSI:arch><NARROW:mode>_1): Use
>         <NARROW:sync_predtab> instead of (implicitly) <CCSI:sync_predtab>.
> (@atomic_compare_and_swap<CCSI:arch><SIDI:mode>_1): Likewise
>         <SIDI:sync_predtab>.  Use <SIDI:cas_cmp_operand> and
>         <SIDI:cas_cmp_str>.
>
> Index: gcc/config/arm/sync.md
> ===================================================================
> --- gcc/config/arm/sync.md      2019-07-01 09:37:07.224524452 +0100
> +++ gcc/config/arm/sync.md      2019-07-05 15:05:09.088609956 +0100
> @@ -201,7 +201,7 @@ (define_insn_and_split "@atomic_compare_
>             (match_operand:SI 7 "const_int_operand")]             ;; mod_f
>            VUNSPEC_ATOMIC_CAS))
>     (clobber (match_scratch:SI 8 "=&r,X,X,X"))]
> -  "<sync_predtab>"
> +  "<NARROW:sync_predtab>"
>    "#"
>    "&& reload_completed"
>    [(const_int 0)]
> @@ -225,14 +225,14 @@ (define_insn_and_split "@atomic_compare_
>          (match_operand:SIDI 2 "mem_noofs_operand" 
> "+Ua,Ua,Ua,Ua"))      ;; memory
>     (set (match_dup 2)
>          (unspec_volatile:SIDI
> -         [(match_operand:SIDI 3 "<cas_cmp_operand>" 
> "<cas_cmp_str>,lIL*h,J,*r") ;; expect
> +         [(match_operand:SIDI 3 "<SIDI:cas_cmp_operand>" 
> "<SIDI:cas_cmp_str>,lIL*h,J,*r") ;; expect
>             (match_operand:SIDI 4 "s_register_operand" "r,r,r,r") ;; 
> desired
>             (match_operand:SI 5 "const_int_operand")              ;; 
> is_weak
>             (match_operand:SI 6 "const_int_operand")              ;; mod_s
>             (match_operand:SI 7 "const_int_operand")]             ;; mod_f
>            VUNSPEC_ATOMIC_CAS))
>     (clobber (match_scratch:SI 8 "=&r,X,X,X"))]
> -  "<sync_predtab>"
> +  "<SIDI:sync_predtab>"
>    "#"
>    "&& reload_completed"
>    [(const_int 0)]

[03/11] [amdgcn] Fix ambiguous .md attribute uses

[Richard Sandiford]

This patch is part of a series that fixes ambiguous attribute
uses in .md files, i.e. cases in which attributes didn't use
<ITER:ATTR> to specify an iterator, and in which <ATTR> could
have different values depending on the iterator chosen.

I think this is a genuine bugfix for the case in which the 1REG_MODE
and 1REG_ALT are different, since previously we would use the 1REG_MODE
for both the comparison and the select, even though the operands being
compared are 1REG_ALT rather than 1REG_MODE.


2019-07-05  Richard Sandiford  <richard.sandiford@arm.com>

gcc/
	* config/gcn/gcn-valu.md
	(vcond<VEC_1REG_MODE:mode><VEC_1REG_ALT:mode>): Use
	gen_vec_cmp<VEC_1REG_ALT:mode>di rather than (implicitly)
	gen_vec_cmp<VEC_1REG_MODE:mode>di.  Explicitly use
	gen_vcond_mask_<VEC_1REG_MODE:mode>di.
	(vcond<VEC_1REG_MODE:mode><VEC_1REG_ALT:mode>_exec): Likewise,
	but using the _exec comparison patterns.
	(vcondu<VEC_1REG_INT_MODE:mode><VEC_1REG_INT_ALT:mode>): Use
	gen_vec_cmp<VEC_1REG_INT_ALT:mode>di rather than (implicitly)
	gen_vec_cmp<VEC_1REG_INT_MODE:mode>di.  Explicitly use
	gen_vcond_mask_<VEC_1REG_INT_MODE:mode>di.
	(vcondu<VEC_1REG_INT_MODE:mode><VEC_1REG_INT_ALT:mode>_exec): Likewise,
	but using the _exec comparison patterns.

Index: gcc/config/gcn/gcn-valu.md
===================================================================
--- gcc/config/gcn/gcn-valu.md	2019-03-08 18:15:37.764736305 +0000
+++ gcc/config/gcn/gcn-valu.md	2019-07-05 15:05:24.076486317 +0100
@@ -2574,10 +2574,10 @@ (define_expand "vcond<VEC_1REG_MODE:mode
   ""
   {
     rtx tmp = gen_reg_rtx (DImode);
-    emit_insn (gen_vec_cmp<mode>di (tmp, operands[3], operands[4],
-				    operands[5]));
-    emit_insn (gen_vcond_mask_<mode>di (operands[0], operands[1], operands[2],
-					tmp));
+    emit_insn (gen_vec_cmp<VEC_1REG_ALT:mode>di
+	       (tmp, operands[3], operands[4], operands[5]));
+    emit_insn (gen_vcond_mask_<VEC_1REG_MODE:mode>di
+	       (operands[0], operands[1], operands[2], tmp));
     DONE;
   })
 
@@ -2592,10 +2592,10 @@ (define_expand "vcond<VEC_1REG_MODE:mode
   ""
   {
     rtx tmp = gen_reg_rtx (DImode);
-    emit_insn (gen_vec_cmp<mode>di_exec (tmp, operands[3], operands[4],
-					 operands[5], operands[6]));
-    emit_insn (gen_vcond_mask_<mode>di (operands[0], operands[1], operands[2],
-					tmp));
+    emit_insn (gen_vec_cmp<VEC_1REG_ALT:mode>di_exec
+	       (tmp, operands[3], operands[4], operands[5], operands[6]));
+    emit_insn (gen_vcond_mask_<VEC_1REG_MODE:mode>di
+	       (operands[0], operands[1], operands[2], tmp));
     DONE;
   })
 
@@ -2609,10 +2609,10 @@ (define_expand "vcondu<VEC_1REG_INT_MODE
   ""
   {
     rtx tmp = gen_reg_rtx (DImode);
-    emit_insn (gen_vec_cmp<mode>di (tmp, operands[3], operands[4],
-				    operands[5]));
-    emit_insn (gen_vcond_mask_<mode>di (operands[0], operands[1], operands[2],
-				        tmp));
+    emit_insn (gen_vec_cmp<VEC_1REG_INT_ALT:mode>di
+	       (tmp, operands[3], operands[4], operands[5]));
+    emit_insn (gen_vcond_mask_<VEC_1REG_INT_MODE:mode>di
+	       (operands[0], operands[1], operands[2], tmp));
     DONE;
   })
 
@@ -2627,10 +2627,10 @@ (define_expand "vcondu<VEC_1REG_INT_MODE
   ""
   {
     rtx tmp = gen_reg_rtx (DImode);
-    emit_insn (gen_vec_cmp<mode>di_exec (tmp, operands[3], operands[4],
-					 operands[5], operands[6]));
-    emit_insn (gen_vcond_mask_<mode>di (operands[0], operands[1], operands[2],
-				        tmp));
+    emit_insn (gen_vec_cmp<VEC_1REG_INT_ALT:mode>di_exec
+	       (tmp, operands[3], operands[4], operands[5], operands[6]));
+    emit_insn (gen_vcond_mask_<VEC_1REG_INT_MODE:mode>di
+	       (operands[0], operands[1], operands[2], tmp));
     DONE;
   })
 

Re: [03/11] [amdgcn] Fix ambiguous .md attribute uses

[Andrew Stubbs]

On 05/07/2019 16:12, Richard Sandiford wrote:
> This patch is part of a series that fixes ambiguous attribute
> uses in .md files, i.e. cases in which attributes didn't use
> <ITER:ATTR> to specify an iterator, and in which <ATTR> could
> have different values depending on the iterator chosen.
> 
> I think this is a genuine bugfix for the case in which the 1REG_MODE
> and 1REG_ALT are different, since previously we would use the 1REG_MODE
> for both the comparison and the select, even though the operands being
> compared are 1REG_ALT rather than 1REG_MODE.
> 
> 
> 2019-07-05  Richard Sandiford  <richard.sandiford@arm.com>
> 
> gcc/
> 	* config/gcn/gcn-valu.md
> 	(vcond<VEC_1REG_MODE:mode><VEC_1REG_ALT:mode>): Use
> 	gen_vec_cmp<VEC_1REG_ALT:mode>di rather than (implicitly)
> 	gen_vec_cmp<VEC_1REG_MODE:mode>di.  Explicitly use
> 	gen_vcond_mask_<VEC_1REG_MODE:mode>di.
> 	(vcond<VEC_1REG_MODE:mode><VEC_1REG_ALT:mode>_exec): Likewise,
> 	but using the _exec comparison patterns.
> 	(vcondu<VEC_1REG_INT_MODE:mode><VEC_1REG_INT_ALT:mode>): Use
> 	gen_vec_cmp<VEC_1REG_INT_ALT:mode>di rather than (implicitly)
> 	gen_vec_cmp<VEC_1REG_INT_MODE:mode>di.  Explicitly use
> 	gen_vcond_mask_<VEC_1REG_INT_MODE:mode>di.
> 	(vcondu<VEC_1REG_INT_MODE:mode><VEC_1REG_INT_ALT:mode>_exec): Likewise,
> 	but using the _exec comparison patterns.

This seems logically correct to me.

Thanks Richard.

Andrew

[04/11] [h8300] Fix ambiguous .md attribute uses

[Richard Sandiford]

This patch is part of a series that fixes ambiguous attribute
uses in .md files, i.e. cases in which attributes didn't use
<ITER:ATTR> to specify an iterator, and in which <ATTR> could
have different values depending on the iterator chosen.

No behavioural change -- produces the same code as before.


2019-07-05  Richard Sandiford  <richard.sandiford@arm.com>

gcc/
	* config/h8300/h8300.md (*push1_h8300hs_<mode>): Explicitly
	specify the mode iterator referenced by <mode>, giving...
	(*push1_h8300hs_<QHI:mode>): ...this.

Index: gcc/config/h8300/h8300.md
===================================================================
--- gcc/config/h8300/h8300.md	2019-07-01 09:37:07.328523581 +0100
+++ gcc/config/h8300/h8300.md	2019-07-05 15:06:32.491921996 +0100
@@ -728,7 +728,7 @@ (define_insn "*pushqi1_h8300"
   "mov.w\\t%T0,@-r7"
   [(set_attr "length" "2")])
 
-(define_insn "*push1_h8300hs_<mode>"
+(define_insn "*push1_h8300hs_<QHI:mode>"
   [(set (mem:QHI
 	(pre_modify:P
 	  (reg:P SP_REG)

Re: [04/11] [h8300] Fix ambiguous .md attribute uses

[Jeff Law]

On 7/5/19 9:13 AM, Richard Sandiford wrote:
> This patch is part of a series that fixes ambiguous attribute
> uses in .md files, i.e. cases in which attributes didn't use
> <ITER:ATTR> to specify an iterator, and in which <ATTR> could
> have different values depending on the iterator chosen.
> 
> No behavioural change -- produces the same code as before.
> 
> 
> 2019-07-05  Richard Sandiford  <richard.sandiford@arm.com>
> 
> gcc/
> 	* config/h8300/h8300.md (*push1_h8300hs_<mode>): Explicitly
> 	specify the mode iterator referenced by <mode>, giving...
> 	(*push1_h8300hs_<QHI:mode>): ...this.
OK
jeff

[05/11] [i386] Fix ambiguous .md attribute uses

[Richard Sandiford]

This patch is part of a series that fixes ambiguous attribute
uses in .md files, i.e. cases in which attributes didn't use
<ITER:ATTR> to specify an iterator, and in which <ATTR> could
have different values depending on the iterator chosen.

No behavioural change except for dropping the unused *andnot<mode>3_bcst
permutations.


2019-07-05  Richard Sandiford  <richard.sandiford@arm.com>

gcc/
	* config/i386/i386.md (*fop_<X87MODEF:mode>_3_i387)
	(l<rounding_insn><MODEF:mode><SWI48:mode>2): Fix ambiguous uses
	of .md attributes.
	* config/i386/sse.md (*avx512pf_gatherpf<mode>sf_mask)
	(*avx512pf_gatherpf<mode>df_mask, *avx512pf_scatterpf<mode>sf_mask)
	(*avx512pf_scatterpf<mode>df_mask, *avx2_gathersi<mode>)
	(*avx2_gathersi<mode>_2, *avx2_gatherdi<mode>)
	(*avx2_gatherdi<mode>_2, *avx2_gatherdi<mode>_3): Likewise.
	(*avx2_gatherdi<mode>_4, *avx512f_gathersi<mode>): Likewise.
	(*avx512f_gathersi<mode>_2, *avx512f_gatherdi<mode>): Likewise.
	(*avx512f_gatherdi<mode>_2, *avx512f_scattersi<mode>): Likewise.
	(*avx512f_scatterdi<mode>): Likewise.
	(*andnot<mode>3_bcst): Fix VI/VI48_AVX512VL typo.

Index: gcc/config/i386/i386.md
===================================================================
--- gcc/config/i386/i386.md	2019-07-03 20:50:46.074319519 +0100
+++ gcc/config/i386/i386.md	2019-07-05 15:05:55.216229452 +0100
@@ -14755,7 +14755,7 @@ (define_insn "*fop_<X87MODEF:mode>_3_i38
 	      ]
 	      (const_string "fop")))
    (set_attr "fp_int_src" "true")
-   (set_attr "mode" "<MODE>")])
+   (set_attr "mode" "<SWI24:MODE>")])
 
 (define_insn "*fop_xf_4_i387"
   [(set (match_operand:XF 0 "register_operand" "=f,f")
@@ -16457,7 +16457,7 @@ (define_expand "l<rounding_insn><MODEF:m
     {
       rtx tmp = gen_reg_rtx (<MODEF:MODE>mode);
 
-      emit_insn (gen_sse4_1_round<mode>2
+      emit_insn (gen_sse4_1_round<MODEF:mode>2
 		 (tmp, operands[1], GEN_INT (ROUND_<ROUNDING>
 					     | ROUND_NO_EXC)));
       emit_insn (gen_fix_trunc<MODEF:mode><SWI48:mode>2
Index: gcc/config/i386/sse.md
===================================================================
--- gcc/config/i386/sse.md	2019-07-03 20:50:46.078319486 +0100
+++ gcc/config/i386/sse.md	2019-07-05 15:05:55.220229422 +0100
@@ -12702,8 +12702,8 @@ (define_insn "*andnot<mode>3"
 	      (const_string "<sseinsnmode>")))])
 
 (define_insn "*andnot<mode>3_bcst"
-  [(set (match_operand:VI 0 "register_operand" "=v")
-	(and:VI
+  [(set (match_operand:VI48_AVX512VL 0 "register_operand" "=v")
+	(and:VI48_AVX512VL
 	  (not:VI48_AVX512VL
 	     (match_operand:VI48_AVX512VL 1 "register_operand" "v"))
 	  (vec_duplicate:VI48_AVX512VL
@@ -18085,7 +18085,7 @@ (define_expand "avx512pf_gatherpf<mode>s
 					operands[3]), UNSPEC_VSIBADDR);
 })
 
-(define_insn "*avx512pf_gatherpf<mode>sf_mask"
+(define_insn "*avx512pf_gatherpf<VI48_512:mode>sf_mask"
   [(unspec
      [(match_operand:<avx512fmaskmode> 0 "register_operand" "Yk")
       (match_operator:<GATHER_SCATTER_SF_MEM_MODE> 5 "vsib_mem_operator"
@@ -18132,7 +18132,7 @@ (define_expand "avx512pf_gatherpf<mode>d
 					operands[3]), UNSPEC_VSIBADDR);
 })
 
-(define_insn "*avx512pf_gatherpf<mode>df_mask"
+(define_insn "*avx512pf_gatherpf<VI4_256_8_512:mode>df_mask"
   [(unspec
      [(match_operand:<avx512fmaskmode> 0 "register_operand" "Yk")
       (match_operator:V8DF 5 "vsib_mem_operator"
@@ -18179,7 +18179,7 @@ (define_expand "avx512pf_scatterpf<mode>
 					operands[3]), UNSPEC_VSIBADDR);
 })
 
-(define_insn "*avx512pf_scatterpf<mode>sf_mask"
+(define_insn "*avx512pf_scatterpf<VI48_512:mode>sf_mask"
   [(unspec
      [(match_operand:<avx512fmaskmode> 0 "register_operand" "Yk")
       (match_operator:<GATHER_SCATTER_SF_MEM_MODE> 5 "vsib_mem_operator"
@@ -18228,7 +18228,7 @@ (define_expand "avx512pf_scatterpf<mode>
 					operands[3]), UNSPEC_VSIBADDR);
 })
 
-(define_insn "*avx512pf_scatterpf<mode>df_mask"
+(define_insn "*avx512pf_scatterpf<VI4_256_8_512:mode>df_mask"
   [(unspec
      [(match_operand:<avx512fmaskmode> 0 "register_operand" "Yk")
       (match_operator:V8DF 5 "vsib_mem_operator"
@@ -21017,7 +21017,7 @@ (define_expand "avx2_gathersi<mode>"
 					operands[5]), UNSPEC_VSIBADDR);
 })
 
-(define_insn "*avx2_gathersi<mode>"
+(define_insn "*avx2_gathersi<VEC_GATHER_MODE:mode>"
   [(set (match_operand:VEC_GATHER_MODE 0 "register_operand" "=&x")
 	(unspec:VEC_GATHER_MODE
 	  [(match_operand:VEC_GATHER_MODE 2 "register_operand" "0")
@@ -21037,7 +21037,7 @@ (define_insn "*avx2_gathersi<mode>"
    (set_attr "prefix" "vex")
    (set_attr "mode" "<sseinsnmode>")])
 
-(define_insn "*avx2_gathersi<mode>_2"
+(define_insn "*avx2_gathersi<VEC_GATHER_MODE:mode>_2"
   [(set (match_operand:VEC_GATHER_MODE 0 "register_operand" "=&x")
 	(unspec:VEC_GATHER_MODE
 	  [(pc)
@@ -21078,7 +21078,7 @@ (define_expand "avx2_gatherdi<mode>"
 					operands[5]), UNSPEC_VSIBADDR);
 })
 
-(define_insn "*avx2_gatherdi<mode>"
+(define_insn "*avx2_gatherdi<VEC_GATHER_MODE:mode>"
   [(set (match_operand:VEC_GATHER_MODE 0 "register_operand" "=&x")
 	(unspec:VEC_GATHER_MODE
 	  [(match_operand:<VEC_GATHER_SRCDI> 2 "register_operand" "0")
@@ -21098,7 +21098,7 @@ (define_insn "*avx2_gatherdi<mode>"
    (set_attr "prefix" "vex")
    (set_attr "mode" "<sseinsnmode>")])
 
-(define_insn "*avx2_gatherdi<mode>_2"
+(define_insn "*avx2_gatherdi<VEC_GATHER_MODE:mode>_2"
   [(set (match_operand:VEC_GATHER_MODE 0 "register_operand" "=&x")
 	(unspec:VEC_GATHER_MODE
 	  [(pc)
@@ -21114,7 +21114,7 @@ (define_insn "*avx2_gatherdi<mode>_2"
    (clobber (match_scratch:VEC_GATHER_MODE 1 "=&x"))]
   "TARGET_AVX2"
 {
-  if (<MODE>mode != <VEC_GATHER_SRCDI>mode)
+  if (<VEC_GATHER_MODE:MODE>mode != <VEC_GATHER_SRCDI>mode)
     return "%M2v<sseintprefix>gatherq<ssemodesuffix>\t{%4, %6, %x0|%x0, %6, %4}";
   return "%M2v<sseintprefix>gatherq<ssemodesuffix>\t{%4, %6, %0|%0, %6, %4}";
 }
@@ -21122,7 +21122,7 @@ (define_insn "*avx2_gatherdi<mode>_2"
    (set_attr "prefix" "vex")
    (set_attr "mode" "<sseinsnmode>")])
 
-(define_insn "*avx2_gatherdi<mode>_3"
+(define_insn "*avx2_gatherdi<VI4F_256:mode>_3"
   [(set (match_operand:<VEC_GATHER_SRCDI> 0 "register_operand" "=&x")
 	(vec_select:<VEC_GATHER_SRCDI>
 	  (unspec:VI4F_256
@@ -21145,7 +21145,7 @@ (define_insn "*avx2_gatherdi<mode>_3"
    (set_attr "prefix" "vex")
    (set_attr "mode" "<sseinsnmode>")])
 
-(define_insn "*avx2_gatherdi<mode>_4"
+(define_insn "*avx2_gatherdi<VI4F_256:mode>_4"
   [(set (match_operand:<VEC_GATHER_SRCDI> 0 "register_operand" "=&x")
 	(vec_select:<VEC_GATHER_SRCDI>
 	  (unspec:VI4F_256
@@ -21187,7 +21187,7 @@ (define_expand "<avx512>_gathersi<mode>"
 					operands[5]), UNSPEC_VSIBADDR);
 })
 
-(define_insn "*avx512f_gathersi<mode>"
+(define_insn "*avx512f_gathersi<VI48F:mode>"
   [(set (match_operand:VI48F 0 "register_operand" "=&v")
 	(unspec:VI48F
 	  [(match_operand:VI48F 1 "register_operand" "0")
@@ -21208,7 +21208,7 @@ (define_insn "*avx512f_gathersi<mode>"
    (set_attr "prefix" "evex")
    (set_attr "mode" "<sseinsnmode>")])
 
-(define_insn "*avx512f_gathersi<mode>_2"
+(define_insn "*avx512f_gathersi<VI48F:mode>_2"
   [(set (match_operand:VI48F 0 "register_operand" "=&v")
 	(unspec:VI48F
 	  [(pc)
@@ -21249,7 +21249,7 @@ (define_expand "<avx512>_gatherdi<mode>"
 					operands[5]), UNSPEC_VSIBADDR);
 })
 
-(define_insn "*avx512f_gatherdi<mode>"
+(define_insn "*avx512f_gatherdi<VI48F:mode>"
   [(set (match_operand:VI48F 0 "register_operand" "=&v")
 	(unspec:VI48F
 	  [(match_operand:<VEC_GATHER_SRCDI> 1 "register_operand" "0")
@@ -21270,7 +21270,7 @@ (define_insn "*avx512f_gatherdi<mode>"
    (set_attr "prefix" "evex")
    (set_attr "mode" "<sseinsnmode>")])
 
-(define_insn "*avx512f_gatherdi<mode>_2"
+(define_insn "*avx512f_gatherdi<VI48F:mode>_2"
   [(set (match_operand:VI48F 0 "register_operand" "=&v")
 	(unspec:VI48F
 	  [(pc)
@@ -21287,9 +21287,9 @@ (define_insn "*avx512f_gatherdi<mode>_2"
 {
   /* %X5 so that we don't emit any *WORD PTR for -masm=intel, as
      gas changed what it requires incompatibly.  */
-  if (<MODE>mode != <VEC_GATHER_SRCDI>mode)
+  if (<VI48F:MODE>mode != <VEC_GATHER_SRCDI>mode)
     {
-      if (<MODE_SIZE> != 64)
+      if (<VI48F:MODE_SIZE> != 64)
 	return "%M3v<sseintprefix>gatherq<ssemodesuffix>\t{%5, %x0%{%1%}|%x0%{%1%}, %X5}";
       else
 	return "%M3v<sseintprefix>gatherq<ssemodesuffix>\t{%5, %t0%{%1%}|%t0%{%1%}, %X5}";
@@ -21318,7 +21318,7 @@ (define_expand "<avx512>_scattersi<mode>
 					operands[4]), UNSPEC_VSIBADDR);
 })
 
-(define_insn "*avx512f_scattersi<mode>"
+(define_insn "*avx512f_scattersi<VI48F:mode>"
   [(set (match_operator:VI48F 5 "vsib_mem_operator"
 	  [(unspec:P
 	     [(match_operand:P 0 "vsib_address_operand" "Tv")
@@ -21356,7 +21356,7 @@ (define_expand "<avx512>_scatterdi<mode>
 					operands[4]), UNSPEC_VSIBADDR);
 })
 
-(define_insn "*avx512f_scatterdi<mode>"
+(define_insn "*avx512f_scatterdi<VI48F:mode>"
   [(set (match_operator:VI48F 5 "vsib_mem_operator"
 	  [(unspec:P
 	     [(match_operand:P 0 "vsib_address_operand" "Tv")

Re: [05/11] [i386] Fix ambiguous .md attribute uses

[Uros Bizjak]

On Fri, Jul 5, 2019 at 5:15 PM Richard Sandiford
<richard.sandiford@arm.com> wrote:
>
> This patch is part of a series that fixes ambiguous attribute
> uses in .md files, i.e. cases in which attributes didn't use
> <ITER:ATTR> to specify an iterator, and in which <ATTR> could
> have different values depending on the iterator chosen.
>
> No behavioural change except for dropping the unused *andnot<mode>3_bcst
> permutations.
>
>
> 2019-07-05  Richard Sandiford  <richard.sandiford@arm.com>
>
> gcc/
>         * config/i386/i386.md (*fop_<X87MODEF:mode>_3_i387)
>         (l<rounding_insn><MODEF:mode><SWI48:mode>2): Fix ambiguous uses
>         of .md attributes.
>         * config/i386/sse.md (*avx512pf_gatherpf<mode>sf_mask)
>         (*avx512pf_gatherpf<mode>df_mask, *avx512pf_scatterpf<mode>sf_mask)
>         (*avx512pf_scatterpf<mode>df_mask, *avx2_gathersi<mode>)
>         (*avx2_gathersi<mode>_2, *avx2_gatherdi<mode>)
>         (*avx2_gatherdi<mode>_2, *avx2_gatherdi<mode>_3): Likewise.
>         (*avx2_gatherdi<mode>_4, *avx512f_gathersi<mode>): Likewise.
>         (*avx512f_gathersi<mode>_2, *avx512f_gatherdi<mode>): Likewise.
>         (*avx512f_gatherdi<mode>_2, *avx512f_scattersi<mode>): Likewise.
>         (*avx512f_scatterdi<mode>): Likewise.
>         (*andnot<mode>3_bcst): Fix VI/VI48_AVX512VL typo.

OK.

Thanks,
Uros.

> Index: gcc/config/i386/i386.md
> ===================================================================
> --- gcc/config/i386/i386.md     2019-07-03 20:50:46.074319519 +0100
> +++ gcc/config/i386/i386.md     2019-07-05 15:05:55.216229452 +0100
> @@ -14755,7 +14755,7 @@ (define_insn "*fop_<X87MODEF:mode>_3_i38
>               ]
>               (const_string "fop")))
>     (set_attr "fp_int_src" "true")
> -   (set_attr "mode" "<MODE>")])
> +   (set_attr "mode" "<SWI24:MODE>")])
>
>  (define_insn "*fop_xf_4_i387"
>    [(set (match_operand:XF 0 "register_operand" "=f,f")
> @@ -16457,7 +16457,7 @@ (define_expand "l<rounding_insn><MODEF:m
>      {
>        rtx tmp = gen_reg_rtx (<MODEF:MODE>mode);
>
> -      emit_insn (gen_sse4_1_round<mode>2
> +      emit_insn (gen_sse4_1_round<MODEF:mode>2
>                  (tmp, operands[1], GEN_INT (ROUND_<ROUNDING>
>                                              | ROUND_NO_EXC)));
>        emit_insn (gen_fix_trunc<MODEF:mode><SWI48:mode>2
> Index: gcc/config/i386/sse.md
> ===================================================================
> --- gcc/config/i386/sse.md      2019-07-03 20:50:46.078319486 +0100
> +++ gcc/config/i386/sse.md      2019-07-05 15:05:55.220229422 +0100
> @@ -12702,8 +12702,8 @@ (define_insn "*andnot<mode>3"
>               (const_string "<sseinsnmode>")))])
>
>  (define_insn "*andnot<mode>3_bcst"
> -  [(set (match_operand:VI 0 "register_operand" "=v")
> -       (and:VI
> +  [(set (match_operand:VI48_AVX512VL 0 "register_operand" "=v")
> +       (and:VI48_AVX512VL
>           (not:VI48_AVX512VL
>              (match_operand:VI48_AVX512VL 1 "register_operand" "v"))
>           (vec_duplicate:VI48_AVX512VL
> @@ -18085,7 +18085,7 @@ (define_expand "avx512pf_gatherpf<mode>s
>                                         operands[3]), UNSPEC_VSIBADDR);
>  })
>
> -(define_insn "*avx512pf_gatherpf<mode>sf_mask"
> +(define_insn "*avx512pf_gatherpf<VI48_512:mode>sf_mask"
>    [(unspec
>       [(match_operand:<avx512fmaskmode> 0 "register_operand" "Yk")
>        (match_operator:<GATHER_SCATTER_SF_MEM_MODE> 5 "vsib_mem_operator"
> @@ -18132,7 +18132,7 @@ (define_expand "avx512pf_gatherpf<mode>d
>                                         operands[3]), UNSPEC_VSIBADDR);
>  })
>
> -(define_insn "*avx512pf_gatherpf<mode>df_mask"
> +(define_insn "*avx512pf_gatherpf<VI4_256_8_512:mode>df_mask"
>    [(unspec
>       [(match_operand:<avx512fmaskmode> 0 "register_operand" "Yk")
>        (match_operator:V8DF 5 "vsib_mem_operator"
> @@ -18179,7 +18179,7 @@ (define_expand "avx512pf_scatterpf<mode>
>                                         operands[3]), UNSPEC_VSIBADDR);
>  })
>
> -(define_insn "*avx512pf_scatterpf<mode>sf_mask"
> +(define_insn "*avx512pf_scatterpf<VI48_512:mode>sf_mask"
>    [(unspec
>       [(match_operand:<avx512fmaskmode> 0 "register_operand" "Yk")
>        (match_operator:<GATHER_SCATTER_SF_MEM_MODE> 5 "vsib_mem_operator"
> @@ -18228,7 +18228,7 @@ (define_expand "avx512pf_scatterpf<mode>
>                                         operands[3]), UNSPEC_VSIBADDR);
>  })
>
> -(define_insn "*avx512pf_scatterpf<mode>df_mask"
> +(define_insn "*avx512pf_scatterpf<VI4_256_8_512:mode>df_mask"
>    [(unspec
>       [(match_operand:<avx512fmaskmode> 0 "register_operand" "Yk")
>        (match_operator:V8DF 5 "vsib_mem_operator"
> @@ -21017,7 +21017,7 @@ (define_expand "avx2_gathersi<mode>"
>                                         operands[5]), UNSPEC_VSIBADDR);
>  })
>
> -(define_insn "*avx2_gathersi<mode>"
> +(define_insn "*avx2_gathersi<VEC_GATHER_MODE:mode>"
>    [(set (match_operand:VEC_GATHER_MODE 0 "register_operand" "=&x")
>         (unspec:VEC_GATHER_MODE
>           [(match_operand:VEC_GATHER_MODE 2 "register_operand" "0")
> @@ -21037,7 +21037,7 @@ (define_insn "*avx2_gathersi<mode>"
>     (set_attr "prefix" "vex")
>     (set_attr "mode" "<sseinsnmode>")])
>
> -(define_insn "*avx2_gathersi<mode>_2"
> +(define_insn "*avx2_gathersi<VEC_GATHER_MODE:mode>_2"
>    [(set (match_operand:VEC_GATHER_MODE 0 "register_operand" "=&x")
>         (unspec:VEC_GATHER_MODE
>           [(pc)
> @@ -21078,7 +21078,7 @@ (define_expand "avx2_gatherdi<mode>"
>                                         operands[5]), UNSPEC_VSIBADDR);
>  })
>
> -(define_insn "*avx2_gatherdi<mode>"
> +(define_insn "*avx2_gatherdi<VEC_GATHER_MODE:mode>"
>    [(set (match_operand:VEC_GATHER_MODE 0 "register_operand" "=&x")
>         (unspec:VEC_GATHER_MODE
>           [(match_operand:<VEC_GATHER_SRCDI> 2 "register_operand" "0")
> @@ -21098,7 +21098,7 @@ (define_insn "*avx2_gatherdi<mode>"
>     (set_attr "prefix" "vex")
>     (set_attr "mode" "<sseinsnmode>")])
>
> -(define_insn "*avx2_gatherdi<mode>_2"
> +(define_insn "*avx2_gatherdi<VEC_GATHER_MODE:mode>_2"
>    [(set (match_operand:VEC_GATHER_MODE 0 "register_operand" "=&x")
>         (unspec:VEC_GATHER_MODE
>           [(pc)
> @@ -21114,7 +21114,7 @@ (define_insn "*avx2_gatherdi<mode>_2"
>     (clobber (match_scratch:VEC_GATHER_MODE 1 "=&x"))]
>    "TARGET_AVX2"
>  {
> -  if (<MODE>mode != <VEC_GATHER_SRCDI>mode)
> +  if (<VEC_GATHER_MODE:MODE>mode != <VEC_GATHER_SRCDI>mode)
>      return "%M2v<sseintprefix>gatherq<ssemodesuffix>\t{%4, %6, %x0|%x0, %6, %4}";
>    return "%M2v<sseintprefix>gatherq<ssemodesuffix>\t{%4, %6, %0|%0, %6, %4}";
>  }
> @@ -21122,7 +21122,7 @@ (define_insn "*avx2_gatherdi<mode>_2"
>     (set_attr "prefix" "vex")
>     (set_attr "mode" "<sseinsnmode>")])
>
> -(define_insn "*avx2_gatherdi<mode>_3"
> +(define_insn "*avx2_gatherdi<VI4F_256:mode>_3"
>    [(set (match_operand:<VEC_GATHER_SRCDI> 0 "register_operand" "=&x")
>         (vec_select:<VEC_GATHER_SRCDI>
>           (unspec:VI4F_256
> @@ -21145,7 +21145,7 @@ (define_insn "*avx2_gatherdi<mode>_3"
>     (set_attr "prefix" "vex")
>     (set_attr "mode" "<sseinsnmode>")])
>
> -(define_insn "*avx2_gatherdi<mode>_4"
> +(define_insn "*avx2_gatherdi<VI4F_256:mode>_4"
>    [(set (match_operand:<VEC_GATHER_SRCDI> 0 "register_operand" "=&x")
>         (vec_select:<VEC_GATHER_SRCDI>
>           (unspec:VI4F_256
> @@ -21187,7 +21187,7 @@ (define_expand "<avx512>_gathersi<mode>"
>                                         operands[5]), UNSPEC_VSIBADDR);
>  })
>
> -(define_insn "*avx512f_gathersi<mode>"
> +(define_insn "*avx512f_gathersi<VI48F:mode>"
>    [(set (match_operand:VI48F 0 "register_operand" "=&v")
>         (unspec:VI48F
>           [(match_operand:VI48F 1 "register_operand" "0")
> @@ -21208,7 +21208,7 @@ (define_insn "*avx512f_gathersi<mode>"
>     (set_attr "prefix" "evex")
>     (set_attr "mode" "<sseinsnmode>")])
>
> -(define_insn "*avx512f_gathersi<mode>_2"
> +(define_insn "*avx512f_gathersi<VI48F:mode>_2"
>    [(set (match_operand:VI48F 0 "register_operand" "=&v")
>         (unspec:VI48F
>           [(pc)
> @@ -21249,7 +21249,7 @@ (define_expand "<avx512>_gatherdi<mode>"
>                                         operands[5]), UNSPEC_VSIBADDR);
>  })
>
> -(define_insn "*avx512f_gatherdi<mode>"
> +(define_insn "*avx512f_gatherdi<VI48F:mode>"
>    [(set (match_operand:VI48F 0 "register_operand" "=&v")
>         (unspec:VI48F
>           [(match_operand:<VEC_GATHER_SRCDI> 1 "register_operand" "0")
> @@ -21270,7 +21270,7 @@ (define_insn "*avx512f_gatherdi<mode>"
>     (set_attr "prefix" "evex")
>     (set_attr "mode" "<sseinsnmode>")])
>
> -(define_insn "*avx512f_gatherdi<mode>_2"
> +(define_insn "*avx512f_gatherdi<VI48F:mode>_2"
>    [(set (match_operand:VI48F 0 "register_operand" "=&v")
>         (unspec:VI48F
>           [(pc)
> @@ -21287,9 +21287,9 @@ (define_insn "*avx512f_gatherdi<mode>_2"
>  {
>    /* %X5 so that we don't emit any *WORD PTR for -masm=intel, as
>       gas changed what it requires incompatibly.  */
> -  if (<MODE>mode != <VEC_GATHER_SRCDI>mode)
> +  if (<VI48F:MODE>mode != <VEC_GATHER_SRCDI>mode)
>      {
> -      if (<MODE_SIZE> != 64)
> +      if (<VI48F:MODE_SIZE> != 64)
>         return "%M3v<sseintprefix>gatherq<ssemodesuffix>\t{%5, %x0%{%1%}|%x0%{%1%}, %X5}";
>        else
>         return "%M3v<sseintprefix>gatherq<ssemodesuffix>\t{%5, %t0%{%1%}|%t0%{%1%}, %X5}";
> @@ -21318,7 +21318,7 @@ (define_expand "<avx512>_scattersi<mode>
>                                         operands[4]), UNSPEC_VSIBADDR);
>  })
>
> -(define_insn "*avx512f_scattersi<mode>"
> +(define_insn "*avx512f_scattersi<VI48F:mode>"
>    [(set (match_operator:VI48F 5 "vsib_mem_operator"
>           [(unspec:P
>              [(match_operand:P 0 "vsib_address_operand" "Tv")
> @@ -21356,7 +21356,7 @@ (define_expand "<avx512>_scatterdi<mode>
>                                         operands[4]), UNSPEC_VSIBADDR);
>  })
>
> -(define_insn "*avx512f_scatterdi<mode>"
> +(define_insn "*avx512f_scatterdi<VI48F:mode>"
>    [(set (match_operator:VI48F 5 "vsib_mem_operator"
>           [(unspec:P
>              [(match_operand:P 0 "vsib_address_operand" "Tv")

[06/11] [mips] Fix ambiguous .md attribute uses

[Richard Sandiford]

This patch is part of a series that fixes ambiguous attribute
uses in .md files, i.e. cases in which attributes didn't use
<ITER:ATTR> to specify an iterator, and in which <ATTR> could
have different values depending on the iterator chosen.

No behavioural change -- produces the same code as before.


2019-07-05  Richard Sandiford  <richard.sandiford@arm.com>

gcc/
	* config/mips/micromips.md (*movep<MOVEP1:mode><MOVEP2:mode>):
	Explicitly use <MOVEP1:MODE> for the mode attribute.

Index: gcc/config/mips/micromips.md
===================================================================
--- gcc/config/mips/micromips.md	2019-03-08 18:15:39.000731609 +0000
+++ gcc/config/mips/micromips.md	2019-07-05 15:07:29.083455268 +0100
@@ -133,5 +133,5 @@ (define_insn "*movep<MOVEP1:mode><MOVEP2
     return "movep\t%2,%0,%z3,%z1";
 }
   [(set_attr "type" "move")
-   (set_attr "mode" "<MODE>")
+   (set_attr "mode" "<MOVEP1:MODE>")
    (set_attr "can_delay" "no")])

Ping: [06/11] [mips] Fix ambiguous .md attribute uses

[Richard Sandiford]

Ping.  I think this is the only one of the series left.

(I should probably have treated the patch as obvious, but it always
seems wrong to do that retrospectively.)

Thanks,
Richard

Richard Sandiford <richard.sandiford@arm.com> writes:
> This patch is part of a series that fixes ambiguous attribute
> uses in .md files, i.e. cases in which attributes didn't use
> <ITER:ATTR> to specify an iterator, and in which <ATTR> could
> have different values depending on the iterator chosen.
>
> No behavioural change -- produces the same code as before.
>
>
> 2019-07-05  Richard Sandiford  <richard.sandiford@arm.com>
>
> gcc/
> 	* config/mips/micromips.md (*movep<MOVEP1:mode><MOVEP2:mode>):
> 	Explicitly use <MOVEP1:MODE> for the mode attribute.
>
> Index: gcc/config/mips/micromips.md
> ===================================================================
> --- gcc/config/mips/micromips.md	2019-03-08 18:15:39.000731609 +0000
> +++ gcc/config/mips/micromips.md	2019-07-05 15:07:29.083455268 +0100
> @@ -133,5 +133,5 @@ (define_insn "*movep<MOVEP1:mode><MOVEP2
>      return "movep\t%2,%0,%z3,%z1";
>  }
>    [(set_attr "type" "move")
> -   (set_attr "mode" "<MODE>")
> +   (set_attr "mode" "<MOVEP1:MODE>")
>     (set_attr "can_delay" "no")])

Re: Ping: [06/11] [mips] Fix ambiguous .md attribute uses

[Jim Wilson]

On 7/12/19 1:16 AM, Richard Sandiford wrote:
> Ping.  I think this is the only one of the series left.
> 
>> 2019-07-05  Richard Sandiford  <richard.sandiford@arm.com>
>> gcc/
>> 	* config/mips/micromips.md (*movep<MOVEP1:mode><MOVEP2:mode>):
>> 	Explicitly use <MOVEP1:MODE> for the mode attribute.

OK.

Jim

[07/11] [riscv] Fix ambiguous .md attribute uses

[Richard Sandiford]

This patch is part of a series that fixes ambiguous attribute
uses in .md files, i.e. cases in which attributes didn't use
<ITER:ATTR> to specify an iterator, and in which <ATTR> could
have different values depending on the iterator chosen.

No behavioural change -- produces the same code as before.


2019-07-05  Richard Sandiford  <richard.sandiford@arm.com>

gcc/
	* config/riscv/pic.md (*local_pic_load_s<mode>)
	(*local_pic_load_u<mode>): Explicitly specify the mode iterator
	referenced by <mode>, giving...
	(*local_pic_load_s<SUBX:mode>, *local_pic_load_u<SUBX:mode>): ...these.
	* config/riscv/riscv.md (*sge<u>_<X:mode><GPR:mode>)
	(*slt<u>_<X:mode><GPR:mode>, *sle<u>_<X:mode><GPR:mode>): Explicitly
	use <X:MODE> for the mode attribute.

Index: gcc/config/riscv/pic.md
===================================================================
--- gcc/config/riscv/pic.md	2019-03-08 18:15:38.084735089 +0000
+++ gcc/config/riscv/pic.md	2019-07-05 15:07:46.695310028 +0100
@@ -29,14 +29,14 @@ (define_insn "*local_pic_load<mode>"
   "<default_load>\t%0,%1"
   [(set (attr "length") (const_int 8))])
 
-(define_insn "*local_pic_load_s<mode>"
+(define_insn "*local_pic_load_s<SUBX:mode>"
   [(set (match_operand:SUPERQI 0 "register_operand" "=r")
 	(sign_extend:SUPERQI (mem:SUBX (match_operand 1 "absolute_symbolic_operand" ""))))]
   "USE_LOAD_ADDRESS_MACRO (operands[1])"
   "<SUBX:load>\t%0,%1"
   [(set (attr "length") (const_int 8))])
 
-(define_insn "*local_pic_load_u<mode>"
+(define_insn "*local_pic_load_u<SUBX:mode>"
   [(set (match_operand:SUPERQI 0 "register_operand" "=r")
 	(zero_extend:SUPERQI (mem:SUBX (match_operand 1 "absolute_symbolic_operand" ""))))]
   "USE_LOAD_ADDRESS_MACRO (operands[1])"
Index: gcc/config/riscv/riscv.md
===================================================================
--- gcc/config/riscv/riscv.md	2019-07-01 09:37:06.632529410 +0100
+++ gcc/config/riscv/riscv.md	2019-07-05 15:07:46.699309994 +0100
@@ -2054,7 +2054,7 @@ (define_insn "*sge<u>_<X:mode><GPR:mode>
   ""
   "slt%i2<u>\t%0,zero,%1"
   [(set_attr "type" "slt")
-   (set_attr "mode" "<MODE>")])
+   (set_attr "mode" "<X:MODE>")])
 
 (define_insn "*slt<u>_<X:mode><GPR:mode>"
   [(set (match_operand:GPR           0 "register_operand" "= r")
@@ -2063,7 +2063,7 @@ (define_insn "*slt<u>_<X:mode><GPR:mode>
   ""
   "slt%i2<u>\t%0,%1,%2"
   [(set_attr "type" "slt")
-   (set_attr "mode" "<MODE>")])
+   (set_attr "mode" "<X:MODE>")])
 
 (define_insn "*sle<u>_<X:mode><GPR:mode>"
   [(set (match_operand:GPR           0 "register_operand" "=r")
@@ -2075,7 +2075,7 @@ (define_insn "*sle<u>_<X:mode><GPR:mode>
   return "slt%i2<u>\t%0,%1,%2";
 }
   [(set_attr "type" "slt")
-   (set_attr "mode" "<MODE>")])
+   (set_attr "mode" "<X:MODE>")])
 
 ;;
 ;;  ....................

Re: [07/11] [riscv] Fix ambiguous .md attribute uses

[Jim Wilson]

On Fri, Jul 5, 2019 at 11:19 PM Richard Sandiford
<richard.sandiford@arm.com> wrote:
> gcc/
>         * config/riscv/pic.md (*local_pic_load_s<mode>)
>         (*local_pic_load_u<mode>): Explicitly specify the mode iterator
>         referenced by <mode>, giving...
>         (*local_pic_load_s<SUBX:mode>, *local_pic_load_u<SUBX:mode>): ...these.
>         * config/riscv/riscv.md (*sge<u>_<X:mode><GPR:mode>)
>         (*slt<u>_<X:mode><GPR:mode>, *sle<u>_<X:mode><GPR:mode>): Explicitly
>         use <X:MODE> for the mode attribute.

OK.

Jim

[08/11] [rs6000] Fix ambiguous .md attribute uses

[Richard Sandiford]

This patch is part of a series that fixes ambiguous attribute
uses in .md files, i.e. cases in which attributes didn't use
<ITER:ATTR> to specify an iterator, and in which <ATTR> could
have different values depending on the iterator chosen.

No behavioural change -- produces the same code as before except
for formatting and line numbers.


2019-07-05  Richard Sandiford  <richard.sandiford@arm.com>

gcc/
	* config/rs6000/rs6000.md (*mov<mode>_update1): Explicitly
	use <SFDF:mode>, <SFDF:MODE>, <SFDF:Ff> and <SFDF:bits> rather than
	leaving the choice between SFDF and P implicit.
	(*mov<mode>_update2): Likewise.
	(*cmp<IBM128:mode>_internal2): Explicitly use <IBM128:MODE>
	rather than leaving the choice betweem IBM128 and GPR implicit.
	(*fix<uns>_trunc<IEEE128:mode><QHSI:mode>2_mem): Explicitly use
	<IEEE128:MODE> rather than leaving the choice between IEEE128 and
	QHSI implicit.
	(AltiVec define_peephole2s): Explicitly use <ALTIVEC_DFORM:MODE>
	rather than leaving the choice between ALTIVEC_DFORM and P implicit.
	* config/rs6000/vsx.md
	(*vsx_ext_<VSX_EXTRACT_I:VS_scalar>_fl_<FL_CONV:mode>)
	(*vsx_ext_<VSX_EXTRACT_I:VS_scalar>_ufl_<FL_CONV:mode>): Explicitly
	use <FL_CONV:VSisa> rather than leaving the choice between FL_CONV
	and VSX_EXTRACT_I implicit.

Index: gcc/config/rs6000/rs6000.md
===================================================================
--- gcc/config/rs6000/rs6000.md	2019-07-03 20:50:46.390316903 +0100
+++ gcc/config/rs6000/rs6000.md	2019-07-05 15:08:06.639145562 +0100
@@ -9312,14 +9312,14 @@ (define_insn "*movqi_update3"
    (set_attr "update" "yes")
    (set_attr "indexed" "yes,no")])
 
-(define_insn "*mov<mode>_update1"
-  [(set (match_operand:SFDF 3 "gpc_reg_operand" "=<Ff>,<Ff>")
+(define_insn "*mov<SFDF:mode>_update1"
+  [(set (match_operand:SFDF 3 "gpc_reg_operand" "=<SFDF:Ff>,<SFDF:Ff>")
 	(mem:SFDF (plus:P (match_operand:P 1 "gpc_reg_operand" "0,0")
 			  (match_operand:P 2 "reg_or_short_operand" "r,I"))))
    (set (match_operand:P 0 "gpc_reg_operand" "=b,b")
 	(plus:P (match_dup 1) (match_dup 2)))]
   "TARGET_HARD_FLOAT && TARGET_UPDATE
-   && (!avoiding_indexed_address_p (<MODE>mode)
+   && (!avoiding_indexed_address_p (<SFDF:MODE>mode)
        || !gpc_reg_operand (operands[2], Pmode))"
   "@
    lf<sd>ux %3,%0,%2
@@ -9327,16 +9327,16 @@ (define_insn "*mov<mode>_update1"
   [(set_attr "type" "fpload")
    (set_attr "update" "yes")
    (set_attr "indexed" "yes,no")
-   (set_attr "size" "<bits>")])
+   (set_attr "size" "<SFDF:bits>")])
 
-(define_insn "*mov<mode>_update2"
+(define_insn "*mov<SFDF:mode>_update2"
   [(set (mem:SFDF (plus:P (match_operand:P 1 "gpc_reg_operand" "0,0")
 			  (match_operand:P 2 "reg_or_short_operand" "r,I")))
-	(match_operand:SFDF 3 "gpc_reg_operand" "<Ff>,<Ff>"))
+	(match_operand:SFDF 3 "gpc_reg_operand" "<SFDF:Ff>,<SFDF:Ff>"))
    (set (match_operand:P 0 "gpc_reg_operand" "=b,b")
 	(plus:P (match_dup 1) (match_dup 2)))]
   "TARGET_HARD_FLOAT && TARGET_UPDATE
-   && (!avoiding_indexed_address_p (<MODE>mode)
+   && (!avoiding_indexed_address_p (<SFDF:MODE>mode)
        || !gpc_reg_operand (operands[2], Pmode))"
   "@
    stf<sd>ux %3,%0,%2
@@ -9344,7 +9344,7 @@ (define_insn "*mov<mode>_update2"
   [(set_attr "type" "fpstore")
    (set_attr "update" "yes")
    (set_attr "indexed" "yes,no")
-   (set_attr "size" "<bits>")])
+   (set_attr "size" "<SFDF:bits>")])
 
 (define_insn "*movsf_update3"
   [(set (match_operand:SF 3 "gpc_reg_operand" "=r,r")
@@ -11558,7 +11558,7 @@ (define_insn "*cmp<mode>_internal1"
   [(set_attr "type" "fpcompare")
    (set_attr "length" "12")])
 
-(define_insn_and_split "*cmp<mode>_internal2"
+(define_insn_and_split "*cmp<IBM128:mode>_internal2"
   [(set (match_operand:CCFP 0 "cc_reg_operand" "=y")
 	(compare:CCFP (match_operand:IBM128 1 "gpc_reg_operand" "d")
 		      (match_operand:IBM128 2 "gpc_reg_operand" "d")))
@@ -11571,7 +11571,7 @@ (define_insn_and_split "*cmp<mode>_inter
     (clobber (match_scratch:DF 9 "=d"))
     (clobber (match_scratch:DF 10 "=d"))
     (clobber (match_scratch:GPR 11 "=b"))]
-  "TARGET_XL_COMPAT && FLOAT128_IBM_P (<MODE>mode)
+  "TARGET_XL_COMPAT && FLOAT128_IBM_P (<IBM128:MODE>mode)
    && TARGET_HARD_FLOAT && TARGET_LONG_DOUBLE_128"
   "#"
   "&& reload_completed"
@@ -11595,10 +11595,14 @@ (define_insn_and_split "*cmp<mode>_inter
   const int lo_word = LONG_DOUBLE_LARGE_FIRST ? GET_MODE_SIZE (DFmode) : 0;
   const int hi_word = LONG_DOUBLE_LARGE_FIRST ? 0 : GET_MODE_SIZE (DFmode);
 
-  operands[5] = simplify_gen_subreg (DFmode, operands[1], <MODE>mode, hi_word);
-  operands[6] = simplify_gen_subreg (DFmode, operands[1], <MODE>mode, lo_word);
-  operands[7] = simplify_gen_subreg (DFmode, operands[2], <MODE>mode, hi_word);
-  operands[8] = simplify_gen_subreg (DFmode, operands[2], <MODE>mode, lo_word);
+  operands[5] = simplify_gen_subreg (DFmode, operands[1],
+				     <IBM128:MODE>mode, hi_word);
+  operands[6] = simplify_gen_subreg (DFmode, operands[1],
+				     <IBM128:MODE>mode, lo_word);
+  operands[7] = simplify_gen_subreg (DFmode, operands[2],
+				     <IBM128:MODE>mode, hi_word);
+  operands[8] = simplify_gen_subreg (DFmode, operands[2],
+				     <IBM128:MODE>mode, lo_word);
   operands[12] = gen_label_rtx ();
   operands[13] = gen_label_rtx ();
   real_inf (&rv);
@@ -13597,7 +13601,7 @@ (define_peephole2
   new_addr = gen_rtx_PLUS (Pmode, add_op0, tmp_reg);
 
   operands[4] = add_op1;
-  operands[5] = change_address (mem, <MODE>mode, new_addr);
+  operands[5] = change_address (mem, <ALTIVEC_DFORM:MODE>mode, new_addr);
 })
 
 ;; Optimize cases were want to do a D-form store on ISA 2.06/2.07 from an
@@ -13633,7 +13637,7 @@ (define_peephole2
   new_addr = gen_rtx_PLUS (Pmode, add_op0, tmp_reg);
 
   operands[4] = add_op1;
-  operands[5] = change_address (mem, <MODE>mode, new_addr);
+  operands[5] = change_address (mem, <ALTIVEC_DFORM:MODE>mode, new_addr);
 })
    
 \f
@@ -14073,7 +14077,7 @@ (define_insn_and_split "*fix<uns>_trunc<
 	(any_fix:QHSI
 	 (match_operand:IEEE128 1 "altivec_register_operand" "v")))
    (clobber (match_scratch:QHSI 2 "=v"))]
-  "TARGET_FLOAT128_HW && FLOAT128_IEEE_P (<MODE>mode)"
+  "TARGET_FLOAT128_HW && FLOAT128_IEEE_P (<IEEE128:MODE>mode)"
   "#"
   "&& reload_completed"
   [(set (match_dup 2)
Index: gcc/config/rs6000/vsx.md
===================================================================
--- gcc/config/rs6000/vsx.md	2019-07-03 20:50:46.390316903 +0100
+++ gcc/config/rs6000/vsx.md	2019-07-05 15:08:06.639145562 +0100
@@ -3828,7 +3828,7 @@ (define_insn_and_split "*vsx_ext_<VSX_EX
 {
   operands[4] = gen_rtx_REG (DImode, REGNO (operands[3]));
 }
-  [(set_attr "isa" "<VSisa>")])
+  [(set_attr "isa" "<FL_CONV:VSisa>")])
 
 (define_insn_and_split "*vsx_ext_<VSX_EXTRACT_I:VS_scalar>_ufl_<FL_CONV:mode>"
   [(set (match_operand:FL_CONV 0 "gpc_reg_operand" "=wa")
@@ -3851,7 +3851,7 @@ (define_insn_and_split "*vsx_ext_<VSX_EX
 {
   operands[4] = gen_rtx_REG (DImode, REGNO (operands[3]));
 }
-  [(set_attr "isa" "<VSisa>")])
+  [(set_attr "isa" "<FL_CONV:VSisa>")])
 
 ;; V4SI/V8HI/V16QI set operation on ISA 3.0
 (define_insn "vsx_set_<mode>_p9"

Re: [08/11] [rs6000] Fix ambiguous .md attribute uses

[Segher Boessenkool]

Hi Richard,

On Fri, Jul 05, 2019 at 04:20:37PM +0100, Richard Sandiford wrote:
> This patch is part of a series that fixes ambiguous attribute
> uses in .md files, i.e. cases in which attributes didn't use
> <ITER:ATTR> to specify an iterator, and in which <ATTR> could
> have different values depending on the iterator chosen.

I have sometimes wondered which iterator is chosen in ambiguous cases.
So I finally looked it up, and the doc says
  The @code{@var{iterator}:} prefix may be omitted, in which case the
  substitution will be attempted for every iterator expansion.

Well ugh :-)  I always thought there was some rule, but nope.

Maybe there should be some way of indicating what iterator you want if
none is mentioned?  For the whole pattern, or some global priority scheme
even?  Changes like (random example)

> -(define_insn "*mov<mode>_update2"
> +(define_insn "*mov<SFDF:mode>_update2"
>    [(set (mem:SFDF (plus:P (match_operand:P 1 "gpc_reg_operand" "0,0")
>  			  (match_operand:P 2 "reg_or_short_operand" "r,I")))
> -	(match_operand:SFDF 3 "gpc_reg_operand" "<Ff>,<Ff>"))
> +	(match_operand:SFDF 3 "gpc_reg_operand" "<SFDF:Ff>,<SFDF:Ff>"))
>     (set (match_operand:P 0 "gpc_reg_operand" "=b,b")
>  	(plus:P (match_dup 1) (match_dup 2)))]
>    "TARGET_HARD_FLOAT && TARGET_UPDATE
> -   && (!avoiding_indexed_address_p (<MODE>mode)
> +   && (!avoiding_indexed_address_p (<SFDF:MODE>mode)

do not make the code more readable.  A rule like "Do not use P if any
other mode iterator is available" would work for us.


(The patch is fine if the whole series is approved, of course).

Thanks,


Segher

Re: [08/11] [rs6000] Fix ambiguous .md attribute uses

[Richard Sandiford]

Segher Boessenkool <segher@kernel.crashing.org> writes:
> Hi Richard,
>
> On Fri, Jul 05, 2019 at 04:20:37PM +0100, Richard Sandiford wrote:
>> This patch is part of a series that fixes ambiguous attribute
>> uses in .md files, i.e. cases in which attributes didn't use
>> <ITER:ATTR> to specify an iterator, and in which <ATTR> could
>> have different values depending on the iterator chosen.
>
> I have sometimes wondered which iterator is chosen in ambiguous cases.
> So I finally looked it up, and the doc says
>   The @code{@var{iterator}:} prefix may be omitted, in which case the
>   substitution will be attempted for every iterator expansion.
>
> Well ugh :-)  I always thought there was some rule, but nope.
>
> Maybe there should be some way of indicating what iterator you want if
> none is mentioned?  For the whole pattern, or some global priority scheme
> even?  Changes like (random example)
>
>> -(define_insn "*mov<mode>_update2"
>> +(define_insn "*mov<SFDF:mode>_update2"
>>    [(set (mem:SFDF (plus:P (match_operand:P 1 "gpc_reg_operand" "0,0")
>>  			  (match_operand:P 2 "reg_or_short_operand" "r,I")))
>> -	(match_operand:SFDF 3 "gpc_reg_operand" "<Ff>,<Ff>"))
>> +	(match_operand:SFDF 3 "gpc_reg_operand" "<SFDF:Ff>,<SFDF:Ff>"))
>>     (set (match_operand:P 0 "gpc_reg_operand" "=b,b")
>>  	(plus:P (match_dup 1) (match_dup 2)))]
>>    "TARGET_HARD_FLOAT && TARGET_UPDATE
>> -   && (!avoiding_indexed_address_p (<MODE>mode)
>> +   && (!avoiding_indexed_address_p (<SFDF:MODE>mode)
>
> do not make the code more readable.  A rule like "Do not use P if any
> other mode iterator is available" would work for us.

Yeah, it's a bit ugly, but I think it's better to be explicit even
for P.  E.g. there are pattern names like:

    vsx_extract_<P:mode>_<VSX_D:mode>_load

in which using that rule and dropping the iterator names would silently
do something unexpected.  (Not a big deal for "*" patterns of course.)

But I think the bigger problem is that attributes tend to grow over
time, and that something that used to be unambiguous can suddenly
become ambiguous without anyone noticing.  E.g. an attribute A might
start with just SI and DI, and so unambiguously refer to P in a PxVECTOR
pattern.  But then it might be useful to add vector modes to A for some
unrelated pattern.

So even if it the order was selectable, it would still be easy to get
things wrong when you're not thinking about it.  And the series is only
forcing an iterator to be specified if there's genuine ambiguity.
E.g. if Ff was specific to floating-point modes, using <Ff> would
still be OK.

> (The patch is fine if the whole series is approved, of course).

Thanks!

Richard

> Thanks,
>
>
> Segher

Re: [08/11] [rs6000] Fix ambiguous .md attribute uses

[Segher Boessenkool]

On Fri, Jul 05, 2019 at 05:26:24PM +0100, Richard Sandiford wrote:
> Segher Boessenkool <segher@kernel.crashing.org> writes:
> > Maybe there should be some way of indicating what iterator you want if
> > none is mentioned?  For the whole pattern, or some global priority scheme
> > even?  Changes like (random example)
> >
> >> -(define_insn "*mov<mode>_update2"
> >> +(define_insn "*mov<SFDF:mode>_update2"
> >>    [(set (mem:SFDF (plus:P (match_operand:P 1 "gpc_reg_operand" "0,0")
> >>  			  (match_operand:P 2 "reg_or_short_operand" "r,I")))
> >> -	(match_operand:SFDF 3 "gpc_reg_operand" "<Ff>,<Ff>"))
> >> +	(match_operand:SFDF 3 "gpc_reg_operand" "<SFDF:Ff>,<SFDF:Ff>"))
> >>     (set (match_operand:P 0 "gpc_reg_operand" "=b,b")
> >>  	(plus:P (match_dup 1) (match_dup 2)))]
> >>    "TARGET_HARD_FLOAT && TARGET_UPDATE
> >> -   && (!avoiding_indexed_address_p (<MODE>mode)
> >> +   && (!avoiding_indexed_address_p (<SFDF:MODE>mode)
> >
> > do not make the code more readable.  A rule like "Do not use P if any
> > other mode iterator is available" would work for us.
> 
> Yeah, it's a bit ugly, but I think it's better to be explicit even
> for P.  E.g. there are pattern names like:
> 
>     vsx_extract_<P:mode>_<VSX_D:mode>_load
> 
> in which using that rule and dropping the iterator names would silently
> do something unexpected.  (Not a big deal for "*" patterns of course.)

This would be

     vsx_extract_<P:mode>_<mode>_load

in that case.  It *can* still be confused, sure.

> But I think the bigger problem is that attributes tend to grow over
> time, and that something that used to be unambiguous can suddenly
> become ambiguous without anyone noticing.  E.g. an attribute A might
> start with just SI and DI, and so unambiguously refer to P in a PxVECTOR
> pattern.  But then it might be useful to add vector modes to A for some
> unrelated pattern.
> 
> So even if it the order was selectable, it would still be easy to get
> things wrong when you're not thinking about it.  And the series is only
> forcing an iterator to be specified if there's genuine ambiguity.

Yeah.  And there aren't very many places being explicit is needed.  Do
you have some estimate how much that "only disallow if actually ambiguous"
helped?  I expected more changes needed :-)

> E.g. if Ff was specific to floating-point modes, using <Ff> would
> still be OK.

Our Ff unfortunately is not for FP modes only, but for modes that can go
in FP registers :-/


Segher

Re: [08/11] [rs6000] Fix ambiguous .md attribute uses

[Richard Sandiford]

Segher Boessenkool <segher@kernel.crashing.org> writes:
> On Fri, Jul 05, 2019 at 05:26:24PM +0100, Richard Sandiford wrote:
>> Segher Boessenkool <segher@kernel.crashing.org> writes:
>> > Maybe there should be some way of indicating what iterator you want if
>> > none is mentioned?  For the whole pattern, or some global priority scheme
>> > even?  Changes like (random example)
>> >
>> >> -(define_insn "*mov<mode>_update2"
>> >> +(define_insn "*mov<SFDF:mode>_update2"
>> >>    [(set (mem:SFDF (plus:P (match_operand:P 1 "gpc_reg_operand" "0,0")
>> >>  			  (match_operand:P 2 "reg_or_short_operand" "r,I")))
>> >> -	(match_operand:SFDF 3 "gpc_reg_operand" "<Ff>,<Ff>"))
>> >> +	(match_operand:SFDF 3 "gpc_reg_operand" "<SFDF:Ff>,<SFDF:Ff>"))
>> >>     (set (match_operand:P 0 "gpc_reg_operand" "=b,b")
>> >>  	(plus:P (match_dup 1) (match_dup 2)))]
>> >>    "TARGET_HARD_FLOAT && TARGET_UPDATE
>> >> -   && (!avoiding_indexed_address_p (<MODE>mode)
>> >> +   && (!avoiding_indexed_address_p (<SFDF:MODE>mode)
>> >
>> > do not make the code more readable.  A rule like "Do not use P if any
>> > other mode iterator is available" would work for us.
>> 
>> Yeah, it's a bit ugly, but I think it's better to be explicit even
>> for P.  E.g. there are pattern names like:
>> 
>>     vsx_extract_<P:mode>_<VSX_D:mode>_load
>> 
>> in which using that rule and dropping the iterator names would silently
>> do something unexpected.  (Not a big deal for "*" patterns of course.)
>
> This would be
>
>      vsx_extract_<P:mode>_<mode>_load
>
> in that case.  It *can* still be confused, sure.
>
>> But I think the bigger problem is that attributes tend to grow over
>> time, and that something that used to be unambiguous can suddenly
>> become ambiguous without anyone noticing.  E.g. an attribute A might
>> start with just SI and DI, and so unambiguously refer to P in a PxVECTOR
>> pattern.  But then it might be useful to add vector modes to A for some
>> unrelated pattern.
>> 
>> So even if it the order was selectable, it would still be easy to get
>> things wrong when you're not thinking about it.  And the series is only
>> forcing an iterator to be specified if there's genuine ambiguity.
>
> Yeah.  And there aren't very many places being explicit is needed.  Do
> you have some estimate how much that "only disallow if actually ambiguous"
> helped?  I expected more changes needed :-)

No real estimate, but I imagine loads :-)  Having two mode iterators
is pretty common, and forcing a qualifier even when there's no
ambiguity (e.g. using vector queries in a vector x P pattern)
would probably be over the top.

And yeah, I was pleasantly surprised how few places needed changing.
The bug-fix to make-work ratio seemed pretty high in the end (but not
for rs6000).

Richard

>
>> E.g. if Ff was specific to floating-point modes, using <Ff> would
>> still be OK.
>
> Our Ff unfortunately is not for FP modes only, but for modes that can go
> in FP registers :-/
>
>
> Segher

[09/11] [s390] Fix ambiguous .md attribute uses

[Richard Sandiford]

This patch is part of a series that fixes ambiguous attribute
uses in .md files, i.e. cases in which attributes didn't use
<ITER:ATTR> to specify an iterator, and in which <ATTR> could
have different values depending on the iterator chosen.

The vx-builtins.md part changes the choice of <mode> from the
implicit <VFCMP:mode> to an explicit <VF_HW:mode> (i.e. from the
mode of the comparison result to the mode of the operands being
compared).  That seemed like the intended behaviour given later
patterns like vec_cmpeq<mode>_cc.

The use of BFP in the s390.md LNDFR pattern looks like a typo,
since the operand to (abs ...) has to have the same mode as the result.
The only effect before this series was to create some extra variants
that would never match, making it harmless apart from very minor code
bloat.


2019-07-05  Richard Sandiford  <richard.sandiford@arm.com>

gcc/
	* config/s390/s390.md (*negabs<FP:mode>2_nocc): Use FP for
	operand 1.
	* config/s390/vx-builtins.md (*vec_cmp<insn_cmp><mode>_cconly):
	Make the choice of <mode> explicit, giving...
	(*vec_cmp<insn_cmp><VF_HW:mode>_cconly): ...this.

Index: gcc/config/s390/s390.md
===================================================================
--- gcc/config/s390/s390.md	2019-07-01 09:37:06.016534568 +0100
+++ gcc/config/s390/s390.md	2019-07-05 15:08:27.018977508 +0100
@@ -8821,8 +8821,8 @@ (define_insn "*negabs<mode>2_cconly"
 
 ; lndfr
 (define_insn "*negabs<mode>2_nocc"
-  [(set (match_operand:FP 0 "register_operand"                  "=f")
-        (neg:FP (abs:FP (match_operand:BFP 1 "register_operand" "<fT0>"))))]
+  [(set (match_operand:FP 0 "register_operand"                 "=f")
+        (neg:FP (abs:FP (match_operand:FP 1 "register_operand" "<fT0>"))))]
   "TARGET_DFP"
   "lndfr\t%0,%1"
   [(set_attr "op_type"  "RRE")
Index: gcc/config/s390/vx-builtins.md
===================================================================
--- gcc/config/s390/vx-builtins.md	2019-04-18 13:28:51.035821466 +0100
+++ gcc/config/s390/vx-builtins.md	2019-07-05 15:08:27.018977508 +0100
@@ -2046,7 +2046,7 @@ (define_insn "*vec_cmphl<VI_HW:mode>_cc"
 ;;
 
 ; vfcesbs, vfcedbs, wfcexbs, vfchsbs, vfchdbs, wfchxbs, vfchesbs, vfchedbs, wfchexbs
-(define_insn "*vec_cmp<insn_cmp><mode>_cconly"
+(define_insn "*vec_cmp<insn_cmp><VF_HW:mode>_cconly"
   [(set (reg:VFCMP CC_REGNUM)
 	(compare:VFCMP (match_operand:VF_HW 0 "register_operand" "v")
 		       (match_operand:VF_HW 1 "register_operand" "v")))

Re: [09/11] [s390] Fix ambiguous .md attribute uses

[Andreas Krebbel]

On 05.07.19 17:22, Richard Sandiford wrote:
> This patch is part of a series that fixes ambiguous attribute
> uses in .md files, i.e. cases in which attributes didn't use
> <ITER:ATTR> to specify an iterator, and in which <ATTR> could
> have different values depending on the iterator chosen.
> 
> The vx-builtins.md part changes the choice of <mode> from the
> implicit <VFCMP:mode> to an explicit <VF_HW:mode> (i.e. from the
> mode of the comparison result to the mode of the operands being
> compared).  That seemed like the intended behaviour given later
> patterns like vec_cmpeq<mode>_cc.
> 
> The use of BFP in the s390.md LNDFR pattern looks like a typo,
> since the operand to (abs ...) has to have the same mode as the result.
> The only effect before this series was to create some extra variants
> that would never match, making it harmless apart from very minor code
> bloat.
> 
> 
> 2019-07-05  Richard Sandiford  <richard.sandiford@arm.com>
> 
> gcc/
> 	* config/s390/s390.md (*negabs<FP:mode>2_nocc): Use FP for
> 	operand 1.
> 	* config/s390/vx-builtins.md (*vec_cmp<insn_cmp><mode>_cconly):
> 	Make the choice of <mode> explicit, giving...
> 	(*vec_cmp<insn_cmp><VF_HW:mode>_cconly): ...this.

Ok. Thanks for fix!

Andreas

[10/11] Use file_location for md_reader's ptr_loc

[Richard Sandiford]

Use file_location for md_reader's ptr_loc.  Also make it public, so that
clients can use the location for error reporting.

I'll apply this once the port changes are in.  (It could go in now,
but it's Friday afternoon...)


2019-07-05  Richard Sandiford  <richard.sandiford@arm.com>

gcc/
	* read-md.h (md_reader::ptr_loc): Moved from read-md.c.
	Use file_location instead of separate fields.
	(md_reader::set_md_ptr_loc): Take a file_location instead of a
	separate filename and line number.
	* read-md.c (ptr_loc): As above.
	(md_reader::copy_md_ptr_loc): Update for new ptr_loc layout.
	(md_reader::fprint_md_ptr_loc): Likewise.
	(md_reader::set_md_ptr_loc): Likewise.  Take a file_location
	instead of a separate filename and line number.
	(md_reader::read_string): Update call accordingly.

Index: gcc/read-md.h
===================================================================
--- gcc/read-md.h	2019-05-12 12:27:15.753897237 +0100
+++ gcc/read-md.h	2019-07-05 15:08:52.698765764 +0100
@@ -148,6 +148,13 @@ struct mapping;
 class md_reader
 {
  public:
+  /* Associates PTR (which can be a string, etc.) with the file location
+     specified by LOC.  */
+  struct ptr_loc {
+    const void *ptr;
+    file_location loc;
+  };
+
   md_reader (bool compact);
   virtual ~md_reader ();
 
@@ -182,7 +189,7 @@ struct mapping;
   void require_word_ws (const char *expected);
   int peek_char (void);
 
-  void set_md_ptr_loc (const void *ptr, const char *filename, int lineno);
+  void set_md_ptr_loc (const void *ptr, file_location);
   const struct ptr_loc *get_md_ptr_loc (const void *ptr);
   void copy_md_ptr_loc (const void *new_ptr, const void *old_ptr);
   void fprint_md_ptr_loc (FILE *outf, const void *ptr);
Index: gcc/read-md.c
===================================================================
--- gcc/read-md.c	2019-03-08 18:15:36.692740380 +0000
+++ gcc/read-md.c	2019-07-05 15:08:52.698765764 +0100
@@ -43,14 +43,6 @@ int have_error = 0;
 #endif /* #ifndef GENERATOR_FILE */
 
 
-/* Associates PTR (which can be a string, etc.) with the file location
-   specified by FILENAME and LINENO.  */
-struct ptr_loc {
-  const void *ptr;
-  const char *filename;
-  int lineno;
-};
-
 /* This callback will be invoked whenever an md include directive is
    processed.  To be used for creation of the dependency file.  */
 void (*include_callback) (const char *);
@@ -94,25 +86,24 @@ leading_ptr_eq_p (const void *def1, cons
   return *(const void *const *) def1 == *(const void *const *) def2;
 }
 
-/* Associate PTR with the file position given by FILENAME and LINENO.  */
+/* Associate PTR with the file position given by FILE_LOC.  */
 
 void
-md_reader::set_md_ptr_loc (const void *ptr, const char *filename, int lineno)
+md_reader::set_md_ptr_loc (const void *ptr, file_location file_loc)
 {
   struct ptr_loc *loc;
 
   loc = (struct ptr_loc *) obstack_alloc (&m_ptr_loc_obstack,
 					  sizeof (struct ptr_loc));
   loc->ptr = ptr;
-  loc->filename = filename;
-  loc->lineno = lineno;
+  loc->loc = file_loc;
   *htab_find_slot (m_ptr_locs, loc, INSERT) = loc;
 }
 
 /* Return the position associated with pointer PTR.  Return null if no
    position was set.  */
 
-const struct ptr_loc *
+const md_reader::ptr_loc *
 md_reader::get_md_ptr_loc (const void *ptr)
 {
   return (const struct ptr_loc *) htab_find (m_ptr_locs, &ptr);
@@ -125,7 +116,7 @@ md_reader::copy_md_ptr_loc (const void *
 {
   const struct ptr_loc *loc = get_md_ptr_loc (old_ptr);
   if (loc != 0)
-    set_md_ptr_loc (new_ptr, loc->filename, loc->lineno);
+    set_md_ptr_loc (new_ptr, loc->loc);
 }
 
 /* If PTR is associated with a known file position, print a #line
@@ -136,7 +127,7 @@ md_reader::fprint_md_ptr_loc (FILE *outf
 {
   const struct ptr_loc *loc = get_md_ptr_loc (ptr);
   if (loc != 0)
-    fprintf (outf, "#line %d \"%s\"\n", loc->lineno, loc->filename);
+    fprintf (outf, "#line %d \"%s\"\n", loc->loc.lineno, loc->loc.filename);
 }
 
 /* Special fprint_md_ptr_loc for writing to STDOUT.  */
@@ -672,7 +663,7 @@ md_reader::read_string (int star_if_brac
 {
   char *stringbuf;
   int saw_paren = 0;
-  int c, old_lineno;
+  int c;
 
   c = read_skip_spaces ();
   if (c == '(')
@@ -681,7 +672,7 @@ md_reader::read_string (int star_if_brac
       c = read_skip_spaces ();
     }
 
-  old_lineno = get_lineno ();
+  file_location loc = get_current_location ();
   if (c == '"')
     stringbuf = read_quoted_string ();
   else if (c == '{')
@@ -704,7 +695,7 @@ md_reader::read_string (int star_if_brac
   if (saw_paren)
     require_char_ws (')');
 
-  set_md_ptr_loc (stringbuf, get_filename (), old_lineno);
+  set_md_ptr_loc (stringbuf, loc);
   return stringbuf;
 }
 

[11/11] Report ambiguous uses of .md attributes

[Richard Sandiford]

This patch reports an error if the .md file has an unscoped
attribute that maps to more than one possible value.

I'll apply this once the port patches are in.


2019-07-05  Richard Sandiford  <richard.sandiford@arm.com>

gcc/
	* read-md.h (md_reader::record_potential_iterator_use): Add a
	file_location parameter.
	* read-rtl.c (attribute_use::loc): New field.
	(map_attr_string): Take a file_location parameter.  Report cases
	in which attributes map to multiple distinct values.
	(apply_attribute_uses): Update call accordingly.
	(md_reader::handle_overloaded_name): Likewise.
	(md_reader::apply_iterator_to_string): Likewise.  Skip empty
	nonnull strings.
	(record_attribute_use): Take a file_location parameter.
	Initialize attribute_use::loc.
	(md_reader::record_potential_iterator_use): Take a file_location
	parameter.  Update call to record_attribute_use.
	(rtx_reader::rtx_alloc_for_name): Update call accordingly.
	(rtx_reader::read_rtx_code): Likewise.
	(rtx_reader::read_rtx_operand): Likewise.  Record a location
	for implicitly-expanded empty strings.

Index: gcc/read-md.h
===================================================================
--- gcc/read-md.h	2019-07-05 15:08:52.698765764 +0100
+++ gcc/read-md.h	2019-07-05 15:09:08.694633872 +0100
@@ -211,8 +211,8 @@ struct mapping;
   rtx copy_rtx_for_iterators (rtx original);
   void read_conditions ();
   void record_potential_iterator_use (struct iterator_group *group,
-				      rtx x, unsigned int index,
-				      const char *name);
+				      file_location loc, rtx x,
+				      unsigned int index, const char *name);
   struct mapping *read_mapping (struct iterator_group *group, htab_t table);
   overloaded_name *handle_overloaded_name (rtx, vec<mapping *> *);
 
Index: gcc/read-rtl.c
===================================================================
--- gcc/read-rtl.c	2019-07-01 09:37:05.780536545 +0100
+++ gcc/read-rtl.c	2019-07-05 15:09:08.694633872 +0100
@@ -106,6 +106,9 @@ struct attribute_use {
   /* The group that describes the use site.  */
   struct iterator_group *group;
 
+  /* The location at which the use occurs.  */
+  file_location loc;
+
   /* The name of the attribute, possibly with an "iterator:" prefix.  */
   const char *value;
 
@@ -361,10 +364,10 @@ find_subst_iter_by_attr (const char *att
 
 /* Map attribute string P to its current value.  Return null if the attribute
    isn't known.  If ITERATOR_OUT is nonnull, store the associated iterator
-   there.  */
+   there.  Report any errors against location LOC.  */
 
 static struct map_value *
-map_attr_string (const char *p, mapping **iterator_out = 0)
+map_attr_string (file_location loc, const char *p, mapping **iterator_out = 0)
 {
   const char *attr;
   struct mapping *iterator;
@@ -372,6 +375,8 @@ map_attr_string (const char *p, mapping
   struct mapping *m;
   struct map_value *v;
   int iterator_name_len;
+  struct map_value *res = NULL;
+  struct mapping *prev = NULL;
 
   /* Peel off any "iterator:" prefix.  Set ATTR to the start of the
      attribute name.  */
@@ -414,13 +419,22 @@ map_attr_string (const char *p, mapping
 	  for (v = m->values; v; v = v->next)
 	    if (v->number == iterator->current_value->number)
 	      {
+		if (res && strcmp (v->string, res->string) != 0)
+		  {
+		    error_at (loc, "ambiguous attribute '%s'; could be"
+			      " '%s' (via '%s:%s') or '%s' (via '%s:%s')",
+			      attr, res->string, prev->name, attr,
+			      v->string, iterator->name, attr);
+		    return v;
+		  }
 		if (iterator_out)
 		  *iterator_out = iterator;
-		return v;
+		prev = iterator;
+		res = v;
 	      }
 	}
     }
-  return NULL;
+  return res;
 }
 
 /* Apply the current iterator values to STRING.  Return the new string
@@ -432,16 +446,17 @@ md_reader::apply_iterator_to_string (con
   char *base, *copy, *p, *start, *end;
   struct map_value *v;
 
-  if (string == 0)
+  if (string == 0 || string[0] == 0)
     return string;
 
+  file_location loc = get_md_ptr_loc (string)->loc;
   base = p = copy = ASTRDUP (string);
   while ((start = strchr (p, '<')) && (end = strchr (start, '>')))
     {
       p = start + 1;
 
       *end = 0;
-      v = map_attr_string (p);
+      v = map_attr_string (loc, p);
       *end = '>';
       if (v == 0)
 	continue;
@@ -572,7 +587,7 @@ apply_attribute_uses (void)
 
   FOR_EACH_VEC_ELT (attribute_uses, i, ause)
     {
-      v = map_attr_string (ause->value);
+      v = map_attr_string (ause->loc, ause->value);
       if (!v)
 	fatal_with_file_and_line ("unknown iterator value `%s'", ause->value);
       ause->group->apply_iterator (ause->x, ause->index,
@@ -656,6 +671,7 @@ md_reader::handle_overloaded_name (rtx o
 
   /* Remove the '@', so that no other code needs to worry about it.  */
   const char *name = XSTR (original, 0);
+  file_location loc = get_md_ptr_loc (name)->loc;
   copy_md_ptr_loc (name + 1, name);
   name += 1;
   XSTR (original, 0) = name;
@@ -672,7 +688,7 @@ md_reader::handle_overloaded_name (rtx o
     {
       *end = 0;
       mapping *iterator;
-      if (!map_attr_string (start + 1, &iterator))
+      if (!map_attr_string (loc, start + 1, &iterator))
 	fatal_with_file_and_line ("unknown iterator `%s'", start + 1);
       *end = '>';
 
@@ -1126,24 +1142,25 @@ record_iterator_use (struct mapping *ite
   iterator_uses.safe_push (iuse);
 }
 
-/* Record that X uses attribute VALUE, which must match a built-in
-   value from group GROUP.  If the use is in an operand of X, INDEX
-   is the index of that operand, otherwise it is ignored.  */
+/* Record that X uses attribute VALUE at location LOC, where VALUE must
+   match a built-in value from group GROUP.  If the use is in an operand
+   of X, INDEX is the index of that operand, otherwise it is ignored.  */
 
 static void
-record_attribute_use (struct iterator_group *group, rtx x,
+record_attribute_use (struct iterator_group *group, file_location loc, rtx x,
 		      unsigned int index, const char *value)
 {
-  struct attribute_use ause = {group, value, x, index};
+  struct attribute_use ause = {group, loc, value, x, index};
   attribute_uses.safe_push (ause);
 }
 
 /* Interpret NAME as either a built-in value, iterator or attribute
    for group GROUP.  X and INDEX are the values to pass to GROUP's
-   apply_iterator callback.  */
+   apply_iterator callback.  LOC is the location of the use.  */
 
 void
 md_reader::record_potential_iterator_use (struct iterator_group *group,
+					  file_location loc,
 					  rtx x, unsigned int index,
 					  const char *name)
 {
@@ -1156,7 +1173,7 @@ md_reader::record_potential_iterator_use
       /* Copy the attribute string into permanent storage, without the
 	 angle brackets around it.  */
       obstack_grow0 (&m_string_obstack, name + 1, len - 2);
-      record_attribute_use (group, x, index,
+      record_attribute_use (group, loc, x, index,
 			    XOBFINISH (&m_string_obstack, char *));
     }
   else
@@ -1540,7 +1557,8 @@ rtx_reader::rtx_alloc_for_name (const ch
       /* Pick the first possible code for now, and record the attribute
 	 use for later.  */
       rtx x = rtx_alloc (check_code_attribute (m));
-      record_attribute_use (&codes, x, 0, deferred_name);
+      record_attribute_use (&codes, get_current_location (),
+			    x, 0, deferred_name);
       return x;
     }
 
@@ -1639,8 +1657,8 @@ rtx_reader::read_rtx_code (const char *c
   c = read_skip_spaces ();
   if (c == ':')
     {
-      read_name (&name);
-      record_potential_iterator_use (&modes, return_rtx, 0, name.string);
+      file_location loc = read_name (&name);
+      record_potential_iterator_use (&modes, loc, return_rtx, 0, name.string);
     }
   else
     unread_char (c);
@@ -1862,6 +1880,7 @@ rtx_reader::read_rtx_operand (rtx return
 		|| GET_CODE (return_rtx) == DEFINE_INSN_AND_SPLIT
 		|| GET_CODE (return_rtx) == DEFINE_INSN_AND_REWRITE))
 	  {
+	    const char *old_stringbuf = stringbuf;
 	    struct obstack *string_obstack = get_string_obstack ();
 	    char line_name[20];
 	    const char *read_md_filename = get_filename ();
@@ -1875,6 +1894,7 @@ rtx_reader::read_rtx_operand (rtx return
 	    sprintf (line_name, ":%d", get_lineno ());
 	    obstack_grow (string_obstack, line_name, strlen (line_name)+1);
 	    stringbuf = XOBFINISH (string_obstack, char *);
+	    copy_md_ptr_loc (stringbuf, old_stringbuf);
 	  }
 
 	/* Find attr-names in the string.  */
@@ -1946,10 +1966,13 @@ rtx_reader::read_rtx_operand (rtx return
     case 'i':
     case 'n':
     case 'p':
-      /* Can be an iterator or an integer constant.  */
-      read_name (&name);
-      record_potential_iterator_use (&ints, return_rtx, idx, name.string);
-      break;
+      {
+	/* Can be an iterator or an integer constant.  */
+	file_location loc = read_name (&name);
+	record_potential_iterator_use (&ints, loc, return_rtx, idx,
+				       name.string);
+	break;
+      }
 
     case 'r':
       read_name (&name);