[Patch, avr] Fix PR 67839 - bit addressable instructions generated for out of range addresses

[Patch, avr] Fix PR 67839 - bit addressable instructions generated for out of range addresses

[Senthil Kumar Selvaraj]

Hi,

  As part of support for io and io_low attributes, the upper bound of
  the range check for low IO and IO addresses was changed from hardcoded
  values to hardcoded_range_end + 1 - GET_MODE_SIZE(mode).

  GCC passes VOID as the mode from genrecog, and GET_MODE_SIZE returns
  0, resulting in the range getting incorrectly extended by a byte.

  Not sure why it was done, as the mode of the operand shouldn't really
  matter when computing the upper bound. In any case, the insns that use 
	the predicate already have a mem:QI wrapping it, and all the bit
  addressable instructions operate on a single IO register only.

  This patch reverts the check back to a hardcoded value, and adds a
  test to prevent regressions.

  No new regression failures. If ok, could someone commit please? I
	don't have commit access.


Regards
Senthil

gcc/ChangeLog

2015-10-05  Senthil Kumar Selvaraj  <senthil_kumar.selvaraj@atmel.com>

	PR target/67839
	* config/avr/predicates.md (low_io_address_operand): Don't
	consider MODE when computing upper bound.
	(io_address_operand): Likewise.


gcc/testsuite/ChangeLog

2015-10-05  Senthil Kumar Selvaraj  <senthil_kumar.selvaraj@atmel.com>

	PR target/67839
	* gcc.target/avr/pr67839.c: New test.



diff --git gcc/config/avr/predicates.md gcc/config/avr/predicates.md
index 2d12bc6..622bc0b 100644
--- gcc/config/avr/predicates.md
+++ gcc/config/avr/predicates.md
@@ -46,7 +46,7 @@
 (define_special_predicate "low_io_address_operand"
   (ior (and (match_code "const_int")
 	    (match_test "IN_RANGE (INTVAL (op) - avr_arch->sfr_offset,
-				   0, 0x20 - GET_MODE_SIZE (mode))"))
+				   0, 0x1F)"))
        (and (match_code "symbol_ref")
 	    (match_test "SYMBOL_REF_FLAGS (op) & SYMBOL_FLAG_IO_LOW"))))
 
@@ -60,7 +60,7 @@
 (define_special_predicate "io_address_operand"
   (ior (and (match_code "const_int")
 	    (match_test "IN_RANGE (INTVAL (op) - avr_arch->sfr_offset,
-				   0, 0x40 - GET_MODE_SIZE (mode))"))
+				   0, 0x3F)"))
        (and (match_code "symbol_ref")
 	    (match_test "SYMBOL_REF_FLAGS (op) & SYMBOL_FLAG_IO"))))
 
diff --git gcc/testsuite/gcc.target/avr/pr67839.c gcc/testsuite/gcc.target/avr/pr67839.c
new file mode 100644
index 0000000..604ab4b
--- /dev/null
+++ gcc/testsuite/gcc.target/avr/pr67839.c
@@ -0,0 +1,29 @@
+/* { dg-do compile } */
+/* { dg-options "-Os" } */
+/* { dg-final { scan-assembler "sbi 0x1f,0" } } */
+/* { dg-final { scan-assembler "cbi 0x1f,0" } } */
+/* { dg-final { scan-assembler-not "sbi 0x20,0" } } */
+/* { dg-final { scan-assembler-not "cbi 0x20,0" } } */
+/* { dg-final { scan-assembler "in r\\d+,__SREG__" } } */
+/* { dg-final { scan-assembler "out __SREG__,r\\d+" } } */
+/* { dg-final { scan-assembler-not "in r\\d+,0x40" } } */
+/* { dg-final { scan-assembler-not "out 0x40, r\\d+" } } */
+
+/* This testcase verifies that SBI/CBI/SBIS/SBIC
+   and IN/OUT instructions are not generated for
+   an IO addresses outside the valid range.
+*/
+#define IO_ADDR(x) (*((volatile char *)x + __AVR_SFR_OFFSET__))
+int main ()
+{
+  IO_ADDR(0x1f) |= 1;
+  IO_ADDR(0x1f) &= 0xFE;
+
+  IO_ADDR(0x20) |= 1;
+  IO_ADDR(0x20) &= 0xFE;
+
+  IO_ADDR(0x3f) = IO_ADDR(0x3f);
+
+  IO_ADDR(0x40) = IO_ADDR(0x40);
+  return 0;
+}

Re: [Patch, avr] Fix PR 67839 - bit addressable instructions generated for out of range addresses

[Senthil Kumar Selvaraj]

Ping!

Regards
Senthil

On Mon, Oct 05, 2015 at 02:30:58PM +0530, Senthil Kumar Selvaraj wrote:
> Hi,
> 
>   As part of support for io and io_low attributes, the upper bound of
>   the range check for low IO and IO addresses was changed from hardcoded
>   values to hardcoded_range_end + 1 - GET_MODE_SIZE(mode).
> 
>   GCC passes VOID as the mode from genrecog, and GET_MODE_SIZE returns
>   0, resulting in the range getting incorrectly extended by a byte.
> 
>   Not sure why it was done, as the mode of the operand shouldn't really
>   matter when computing the upper bound. In any case, the insns that use 
> 	the predicate already have a mem:QI wrapping it, and all the bit
>   addressable instructions operate on a single IO register only.
> 
>   This patch reverts the check back to a hardcoded value, and adds a
>   test to prevent regressions.
> 
>   No new regression failures. If ok, could someone commit please? I
> 	don't have commit access.
> 
> 
> Regards
> Senthil
> 
> gcc/ChangeLog
> 
> 2015-10-05  Senthil Kumar Selvaraj  <senthil_kumar.selvaraj@atmel.com>
> 
> 	PR target/67839
> 	* config/avr/predicates.md (low_io_address_operand): Don't
> 	consider MODE when computing upper bound.
> 	(io_address_operand): Likewise.
> 
> 
> gcc/testsuite/ChangeLog
> 
> 2015-10-05  Senthil Kumar Selvaraj  <senthil_kumar.selvaraj@atmel.com>
> 
> 	PR target/67839
> 	* gcc.target/avr/pr67839.c: New test.
> 
> 
> 
> diff --git gcc/config/avr/predicates.md gcc/config/avr/predicates.md
> index 2d12bc6..622bc0b 100644
> --- gcc/config/avr/predicates.md
> +++ gcc/config/avr/predicates.md
> @@ -46,7 +46,7 @@
>  (define_special_predicate "low_io_address_operand"
>    (ior (and (match_code "const_int")
>  	    (match_test "IN_RANGE (INTVAL (op) - avr_arch->sfr_offset,
> -				   0, 0x20 - GET_MODE_SIZE (mode))"))
> +				   0, 0x1F)"))
>         (and (match_code "symbol_ref")
>  	    (match_test "SYMBOL_REF_FLAGS (op) & SYMBOL_FLAG_IO_LOW"))))
>  
> @@ -60,7 +60,7 @@
>  (define_special_predicate "io_address_operand"
>    (ior (and (match_code "const_int")
>  	    (match_test "IN_RANGE (INTVAL (op) - avr_arch->sfr_offset,
> -				   0, 0x40 - GET_MODE_SIZE (mode))"))
> +				   0, 0x3F)"))
>         (and (match_code "symbol_ref")
>  	    (match_test "SYMBOL_REF_FLAGS (op) & SYMBOL_FLAG_IO"))))
>  
> diff --git gcc/testsuite/gcc.target/avr/pr67839.c gcc/testsuite/gcc.target/avr/pr67839.c
> new file mode 100644
> index 0000000..604ab4b
> --- /dev/null
> +++ gcc/testsuite/gcc.target/avr/pr67839.c
> @@ -0,0 +1,29 @@
> +/* { dg-do compile } */
> +/* { dg-options "-Os" } */
> +/* { dg-final { scan-assembler "sbi 0x1f,0" } } */
> +/* { dg-final { scan-assembler "cbi 0x1f,0" } } */
> +/* { dg-final { scan-assembler-not "sbi 0x20,0" } } */
> +/* { dg-final { scan-assembler-not "cbi 0x20,0" } } */
> +/* { dg-final { scan-assembler "in r\\d+,__SREG__" } } */
> +/* { dg-final { scan-assembler "out __SREG__,r\\d+" } } */
> +/* { dg-final { scan-assembler-not "in r\\d+,0x40" } } */
> +/* { dg-final { scan-assembler-not "out 0x40, r\\d+" } } */
> +
> +/* This testcase verifies that SBI/CBI/SBIS/SBIC
> +   and IN/OUT instructions are not generated for
> +   an IO addresses outside the valid range.
> +*/
> +#define IO_ADDR(x) (*((volatile char *)x + __AVR_SFR_OFFSET__))
> +int main ()
> +{
> +  IO_ADDR(0x1f) |= 1;
> +  IO_ADDR(0x1f) &= 0xFE;
> +
> +  IO_ADDR(0x20) |= 1;
> +  IO_ADDR(0x20) &= 0xFE;
> +
> +  IO_ADDR(0x3f) = IO_ADDR(0x3f);
> +
> +  IO_ADDR(0x40) = IO_ADDR(0x40);
> +  return 0;
> +}

Re: [Patch, avr] Fix PR 67839 - bit addressable instructions generated for out of range addresses

[Senthil Kumar Selvaraj]

Ping!

Regards
Senthil
On Fri, Oct 16, 2015 at 03:17:17PM +0530, Senthil Kumar Selvaraj wrote:
> Ping!
> 
> Regards
> Senthil
> 
> On Mon, Oct 05, 2015 at 02:30:58PM +0530, Senthil Kumar Selvaraj wrote:
> > Hi,
> > 
> >   As part of support for io and io_low attributes, the upper bound of
> >   the range check for low IO and IO addresses was changed from hardcoded
> >   values to hardcoded_range_end + 1 - GET_MODE_SIZE(mode).
> > 
> >   GCC passes VOID as the mode from genrecog, and GET_MODE_SIZE returns
> >   0, resulting in the range getting incorrectly extended by a byte.
> > 
> >   Not sure why it was done, as the mode of the operand shouldn't really
> >   matter when computing the upper bound. In any case, the insns that use 
> > 	the predicate already have a mem:QI wrapping it, and all the bit
> >   addressable instructions operate on a single IO register only.
> > 
> >   This patch reverts the check back to a hardcoded value, and adds a
> >   test to prevent regressions.
> > 
> >   No new regression failures. If ok, could someone commit please? I
> > 	don't have commit access.
> > 
> > 
> > Regards
> > Senthil
> > 
> > gcc/ChangeLog
> > 
> > 2015-10-05  Senthil Kumar Selvaraj  <senthil_kumar.selvaraj@atmel.com>
> > 
> > 	PR target/67839
> > 	* config/avr/predicates.md (low_io_address_operand): Don't
> > 	consider MODE when computing upper bound.
> > 	(io_address_operand): Likewise.
> > 
> > 
> > gcc/testsuite/ChangeLog
> > 
> > 2015-10-05  Senthil Kumar Selvaraj  <senthil_kumar.selvaraj@atmel.com>
> > 
> > 	PR target/67839
> > 	* gcc.target/avr/pr67839.c: New test.
> > 
> > 
> > 
> > diff --git gcc/config/avr/predicates.md gcc/config/avr/predicates.md
> > index 2d12bc6..622bc0b 100644
> > --- gcc/config/avr/predicates.md
> > +++ gcc/config/avr/predicates.md
> > @@ -46,7 +46,7 @@
> >  (define_special_predicate "low_io_address_operand"
> >    (ior (and (match_code "const_int")
> >  	    (match_test "IN_RANGE (INTVAL (op) - avr_arch->sfr_offset,
> > -				   0, 0x20 - GET_MODE_SIZE (mode))"))
> > +				   0, 0x1F)"))
> >         (and (match_code "symbol_ref")
> >  	    (match_test "SYMBOL_REF_FLAGS (op) & SYMBOL_FLAG_IO_LOW"))))
> >  
> > @@ -60,7 +60,7 @@
> >  (define_special_predicate "io_address_operand"
> >    (ior (and (match_code "const_int")
> >  	    (match_test "IN_RANGE (INTVAL (op) - avr_arch->sfr_offset,
> > -				   0, 0x40 - GET_MODE_SIZE (mode))"))
> > +				   0, 0x3F)"))
> >         (and (match_code "symbol_ref")
> >  	    (match_test "SYMBOL_REF_FLAGS (op) & SYMBOL_FLAG_IO"))))
> >  
> > diff --git gcc/testsuite/gcc.target/avr/pr67839.c gcc/testsuite/gcc.target/avr/pr67839.c
> > new file mode 100644
> > index 0000000..604ab4b
> > --- /dev/null
> > +++ gcc/testsuite/gcc.target/avr/pr67839.c
> > @@ -0,0 +1,29 @@
> > +/* { dg-do compile } */
> > +/* { dg-options "-Os" } */
> > +/* { dg-final { scan-assembler "sbi 0x1f,0" } } */
> > +/* { dg-final { scan-assembler "cbi 0x1f,0" } } */
> > +/* { dg-final { scan-assembler-not "sbi 0x20,0" } } */
> > +/* { dg-final { scan-assembler-not "cbi 0x20,0" } } */
> > +/* { dg-final { scan-assembler "in r\\d+,__SREG__" } } */
> > +/* { dg-final { scan-assembler "out __SREG__,r\\d+" } } */
> > +/* { dg-final { scan-assembler-not "in r\\d+,0x40" } } */
> > +/* { dg-final { scan-assembler-not "out 0x40, r\\d+" } } */
> > +
> > +/* This testcase verifies that SBI/CBI/SBIS/SBIC
> > +   and IN/OUT instructions are not generated for
> > +   an IO addresses outside the valid range.
> > +*/
> > +#define IO_ADDR(x) (*((volatile char *)x + __AVR_SFR_OFFSET__))
> > +int main ()
> > +{
> > +  IO_ADDR(0x1f) |= 1;
> > +  IO_ADDR(0x1f) &= 0xFE;
> > +
> > +  IO_ADDR(0x20) |= 1;
> > +  IO_ADDR(0x20) &= 0xFE;
> > +
> > +  IO_ADDR(0x3f) = IO_ADDR(0x3f);
> > +
> > +  IO_ADDR(0x40) = IO_ADDR(0x40);
> > +  return 0;
> > +}

Re: [Patch, avr] Fix PR 67839 - bit addressable instructions generated for out of range addresses

[Denis Chertykov]

2015-10-28 9:28 GMT+03:00 Senthil Kumar Selvaraj
<senthil_kumar.selvaraj@atmel.com>:
>
> Ping!
>
> Regards
> Senthil
> On Fri, Oct 16, 2015 at 03:17:17PM +0530, Senthil Kumar Selvaraj wrote:
> > Ping!
> >
> > Regards
> > Senthil
> >
> > On Mon, Oct 05, 2015 at 02:30:58PM +0530, Senthil Kumar Selvaraj wrote:
> > > Hi,
> > >
> > >   As part of support for io and io_low attributes, the upper bound of
> > >   the range check for low IO and IO addresses was changed from hardcoded
> > >   values to hardcoded_range_end + 1 - GET_MODE_SIZE(mode).
> > >
> > >   GCC passes VOID as the mode from genrecog, and GET_MODE_SIZE returns
> > >   0, resulting in the range getting incorrectly extended by a byte.
> > >
> > >   Not sure why it was done, as the mode of the operand shouldn't really
> > >   matter when computing the upper bound. In any case, the insns that use
> > >     the predicate already have a mem:QI wrapping it, and all the bit
> > >   addressable instructions operate on a single IO register only.
> > >
> > >   This patch reverts the check back to a hardcoded value, and adds a
> > >   test to prevent regressions.
> > >
> > >   No new regression failures. If ok, could someone commit please? I
> > >     don't have commit access.
> > >
> > >
> > > Regards
> > > Senthil
> > >
> > > gcc/ChangeLog
> > >
> > > 2015-10-05  Senthil Kumar Selvaraj  <senthil_kumar.selvaraj@atmel.com>
> > >
> > >     PR target/67839
> > >     * config/avr/predicates.md (low_io_address_operand): Don't
> > >     consider MODE when computing upper bound.
> > >     (io_address_operand): Likewise.
> > >
> > >
> > > gcc/testsuite/ChangeLog
> > >
> > > 2015-10-05  Senthil Kumar Selvaraj  <senthil_kumar.selvaraj@atmel.com>
> > >
> > >     PR target/67839
> > >     * gcc.target/avr/pr67839.c: New test.
> > >

Committed.

Denis