diff mbox

[ARM] PR target/79871: Clean up lane/constant bounds checking errors for translation

Message ID 58D4FF86.8080902@foss.arm.com
State New
Headers show

Commit Message

Kyrill Tkachov March 24, 2017, 11:14 a.m. UTC 
Hi all,

This PR complains that the bounds checking error strings contain a string placeholder for "constant" or "lane"
which makes it hard for translators (who may want to move words around in a different language for syntactical reasons).

This patch cleans that up. The bunching up of common functionality between neon_lane_bounds and arm_const_bounds was a bit
dubious in any case as arm_const_bounds never passed down a non-NULL tree anyway, so one of the paths of bonds_check was
always used in only the neon_lane_bounds case anyway.

I also add some function comments and use IN_RANGE for range checking.
Bootstrapped and tested on arm-none-linux-gnueabihf.

Ok for trunk? (I believe such translation improvements fall under the documentation rule at this stage).

Thanks,
Kyrill

2017-03-24  Kyrylo Tkachov  <kyrylo.tkachov@arm.com>

     PR target/79871
     * config/arm/arm.c (bounds_check): Delete.
     (neon_lane_bounds): Adjust.  Make sure error string template
     doesn't use string placeholder.
     (arm_const_bounds): Likewise.

Comments

Kyrill Tkachov April 10, 2017, 10:56 a.m. UTC | #1 
Ping.

https://gcc.gnu.org/ml/gcc-patches/2017-03/msg01271.html

Thanks,
Kyrill

On 24/03/17 11:14, Kyrill Tkachov wrote:
> Hi all,
>
> This PR complains that the bounds checking error strings contain a string placeholder for "constant" or "lane"
> which makes it hard for translators (who may want to move words around in a different language for syntactical reasons).
>
> This patch cleans that up. The bunching up of common functionality between neon_lane_bounds and arm_const_bounds was a bit
> dubious in any case as arm_const_bounds never passed down a non-NULL tree anyway, so one of the paths of bonds_check was
> always used in only the neon_lane_bounds case anyway.
>
> I also add some function comments and use IN_RANGE for range checking.
> Bootstrapped and tested on arm-none-linux-gnueabihf.
>
> Ok for trunk? (I believe such translation improvements fall under the documentation rule at this stage).
>
> Thanks,
> Kyrill
>
> 2017-03-24  Kyrylo Tkachov  <kyrylo.tkachov@arm.com>
>
>     PR target/79871
>     * config/arm/arm.c (bounds_check): Delete.
>     (neon_lane_bounds): Adjust.  Make sure error string template
>     doesn't use string placeholder.
>     (arm_const_bounds): Likewise.
diff mbox

Patch

commit 102b86a782297c725c4796c4dd36d33fdf024ee7
Author: Kyrylo Tkachov <kyrylo.tkachov@arm.com>
Date:   Thu Mar 23 10:50:32 2017 +0000

    PR target/79871: Clean up lane/constant bounds checking errors for translation

diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c
index 7b2d3d5..98059da 100644
--- a/gcc/config/arm/arm.c
+++ b/gcc/config/arm/arm.c
@@ -12186,13 +12186,16 @@  neon_expand_vector_init (rtx target, rtx vals)
   emit_move_insn (target, mem);
 }
 
-/* Ensure OPERAND lies between LOW (inclusive) and HIGH (exclusive).  Raise
-   ERR if it doesn't.  EXP indicates the source location, which includes the
-   inlining history for intrinsics.  */
+/* Bounds-check lanes.
+   Ensure OPERAND lies between LOW (inclusive) and HIGH (exclusive).  Emit an
+   an error if it doesn't.  EXP indicates the source location, which includes
+   the inlining history for intrinsics.
+   We don't unify this and arm_const_bounds because the error string needs to
+   explicity contain "constant" or "lane" for translation purposes.  */
 
-static void
-bounds_check (rtx operand, HOST_WIDE_INT low, HOST_WIDE_INT high,
-	      const_tree exp, const char *desc)
+void
+neon_lane_bounds (rtx operand, HOST_WIDE_INT low, HOST_WIDE_INT high,
+		  const_tree exp)
 {
   HOST_WIDE_INT lane;
 
@@ -12200,31 +12203,34 @@  bounds_check (rtx operand, HOST_WIDE_INT low, HOST_WIDE_INT high,
 
   lane = INTVAL (operand);
 
-  if (lane < low || lane >= high)
+  if (!IN_RANGE (lane, low, high - 1))
     {
       if (exp)
-	error ("%K%s %wd out of range %wd - %wd",
-	       exp, desc, lane, low, high - 1);
+	error ("%Klane %wd out of range %wd - %wd",
+		exp, lane, low, high - 1);
       else
-	error ("%s %wd out of range %wd - %wd", desc, lane, low, high - 1);
+	error ("lane %wd out of range %wd - %wd",
+		lane, low, high - 1);
     }
 }
 
-/* Bounds-check lanes.  */
+/* Bounds-check constants.
+   Ensure OPERAND lies between LOW (inclusive) and HIGH (exclusive).  Emit an
+   an error if it doesn't.  See neon_lane_bounds comment for
+   translation comment.  */
 
 void
-neon_lane_bounds (rtx operand, HOST_WIDE_INT low, HOST_WIDE_INT high,
-		  const_tree exp)
+arm_const_bounds (rtx operand, HOST_WIDE_INT low, HOST_WIDE_INT high)
 {
-  bounds_check (operand, low, high, exp, "lane");
-}
+  HOST_WIDE_INT constant;
 
-/* Bounds-check constants.  */
+  gcc_assert (CONST_INT_P (operand));
 
-void
-arm_const_bounds (rtx operand, HOST_WIDE_INT low, HOST_WIDE_INT high)
-{
-  bounds_check (operand, low, high, NULL_TREE, "constant");
+  constant = INTVAL (operand);
+
+  if (!IN_RANGE (constant, low, high - 1))
+    error ("constant %wd out of range %wd - %wd",
+	    constant, low, high - 1);
 }
 
 HOST_WIDE_INT