| Message ID | 5804F919.3020407@foss.arm.com |
|---|---|
| State | New |
| Headers | show |
Ping. https://gcc.gnu.org/ml/gcc-patches/2016-10/msg01321.html Thanks, Kyrill On 17/10/16 17:15, Kyrill Tkachov wrote: > Hi all, > > For the attached testcase the code ends up trying to extract bits outside the range of the normal register > widths. The aarch64 patterns for ubfz and tbnz end up accepting such operands and emitting invalid assembly > such as 'ubfx x18,x2,192,32' > > The solution is to add proper predicates and guards to the operands of the zero_extract operations that are going on. > I had a look at all the other patterns in aarch64 that generate/use zero_extract and they all have guards on their > operands in one form or another to avoid them accessing an area that is out of range. > > With this patch the testcase compiles and assembles fine. > > Bootstrapped and tested on aarch64-none-linux-gnu. > > Ok for trunk? > > Thanks, > Kyrill > > 2016-10-17 Kyrylo Tkachov <kyrylo.tkachov@arm.com> > > PR target/77822 > * config/aarch64/aarch64.md (*tb<optab><mode>1): Use > aarch64_simd_shift_imm_<mode> predicate for operand 1. > (<optab>, ANY_EXTRACT): Use tighter predicates on operands 2 and 3 > to restrict them to an appropriate range and add FAIL check if the > region they specify is out of range. Delete useless constraint > strings. > (*<optab><mode>, ANY_EXTRACT): Add appropriate predicates on operands > 2 and 3 to restrict their range and add pattern predicate. > > 2016-10-17 Kyrylo Tkachov <kyrylo.tkachov@arm.com> > > PR target/77822
On 24/10/16 12:29, Kyrill Tkachov wrote: > Ping. > https://gcc.gnu.org/ml/gcc-patches/2016-10/msg01321.html > I just noticed my original ChangeLog entry was truncated. It is 2016-10-04 Kyrylo Tkachov <kyrylo.tkachov@arm.com> PR target/77822 * config/aarch64/aarch64.md (*tb<optab><mode>1): Use aarch64_simd_shift_imm_<mode> predicate for operand 1. (<optab>, ANY_EXTRACT): Use tighter predicates on operands 2 and 3 to restrict them to an appropriate range and add FAIL check if the region they specify is out of range. Delete useless constraint strings. (*<optab><mode>, ANY_EXTRACT): Add appropriate predicates on operands 2 and 3 to restrict their range and add pattern predicate. 2016-10-04 Kyrylo Tkachov <kyrylo.tkachov@arm.com> PR target/77822 * g++.dg/torture/pr77822.C: New test. Kyrill > > On 17/10/16 17:15, Kyrill Tkachov wrote: >> Hi all, >> >> For the attached testcase the code ends up trying to extract bits outside the range of the normal register >> widths. The aarch64 patterns for ubfz and tbnz end up accepting such operands and emitting invalid assembly >> such as 'ubfx x18,x2,192,32' >> >> The solution is to add proper predicates and guards to the operands of the zero_extract operations that are going on. >> I had a look at all the other patterns in aarch64 that generate/use zero_extract and they all have guards on their >> operands in one form or another to avoid them accessing an area that is out of range. >> >> With this patch the testcase compiles and assembles fine. >> >> Bootstrapped and tested on aarch64-none-linux-gnu. >> >> Ok for trunk? >> >> Thanks, >> Kyrill >> >> 2016-10-17 Kyrylo Tkachov <kyrylo.tkachov@arm.com> >> >> PR target/77822 >> * config/aarch64/aarch64.md (*tb<optab><mode>1): Use >> aarch64_simd_shift_imm_<mode> predicate for operand 1. >> (<optab>, ANY_EXTRACT): Use tighter predicates on operands 2 and 3 >> to restrict them to an appropriate range and add FAIL check if the >> region they specify is out of range. Delete useless constraint >> strings. >> (*<optab><mode>, ANY_EXTRACT): Add appropriate predicates on operands >> 2 and 3 to restrict their range and add pattern predicate. >> >> 2016-10-17 Kyrylo Tkachov <kyrylo.tkachov@arm.com> >> >> PR target/77822 >
Ping. Thanks, Kyrill On 24/10/16 14:12, Kyrill Tkachov wrote: > > On 24/10/16 12:29, Kyrill Tkachov wrote: >> Ping. >> https://gcc.gnu.org/ml/gcc-patches/2016-10/msg01321.html >> > > I just noticed my original ChangeLog entry was truncated. > It is > 2016-10-04 Kyrylo Tkachov <kyrylo.tkachov@arm.com> > > PR target/77822 > * config/aarch64/aarch64.md (*tb<optab><mode>1): Use > aarch64_simd_shift_imm_<mode> predicate for operand 1. > (<optab>, ANY_EXTRACT): Use tighter predicates on operands 2 and 3 > to restrict them to an appropriate range and add FAIL check if the > region they specify is out of range. Delete useless constraint > strings. > (*<optab><mode>, ANY_EXTRACT): Add appropriate predicates on operands > 2 and 3 to restrict their range and add pattern predicate. > > 2016-10-04 Kyrylo Tkachov <kyrylo.tkachov@arm.com> > > PR target/77822 > * g++.dg/torture/pr77822.C: New test. > > Kyrill > >> >> On 17/10/16 17:15, Kyrill Tkachov wrote: >>> Hi all, >>> >>> For the attached testcase the code ends up trying to extract bits outside the range of the normal register >>> widths. The aarch64 patterns for ubfz and tbnz end up accepting such operands and emitting invalid assembly >>> such as 'ubfx x18,x2,192,32' >>> >>> The solution is to add proper predicates and guards to the operands of the zero_extract operations that are going on. >>> I had a look at all the other patterns in aarch64 that generate/use zero_extract and they all have guards on their >>> operands in one form or another to avoid them accessing an area that is out of range. >>> >>> With this patch the testcase compiles and assembles fine. >>> >>> Bootstrapped and tested on aarch64-none-linux-gnu. >>> >>> Ok for trunk? >>> >>> Thanks, >>> Kyrill >>> >>> 2016-10-17 Kyrylo Tkachov <kyrylo.tkachov@arm.com> >>> >>> PR target/77822 >>> * config/aarch64/aarch64.md (*tb<optab><mode>1): Use >>> aarch64_simd_shift_imm_<mode> predicate for operand 1. >>> (<optab>, ANY_EXTRACT): Use tighter predicates on operands 2 and 3 >>> to restrict them to an appropriate range and add FAIL check if the >>> region they specify is out of range. Delete useless constraint >>> strings. >>> (*<optab><mode>, ANY_EXTRACT): Add appropriate predicates on operands >>> 2 and 3 to restrict their range and add pattern predicate. >>> >>> 2016-10-17 Kyrylo Tkachov <kyrylo.tkachov@arm.com> >>> >>> PR target/77822 >> >
Ping. Thanks, Kyrill On 31/10/16 12:10, Kyrill Tkachov wrote: > Ping. > > Thanks, > Kyrill > > On 24/10/16 14:12, Kyrill Tkachov wrote: >> >> On 24/10/16 12:29, Kyrill Tkachov wrote: >>> Ping. >>> https://gcc.gnu.org/ml/gcc-patches/2016-10/msg01321.html >>> >> >> I just noticed my original ChangeLog entry was truncated. >> It is >> 2016-10-04 Kyrylo Tkachov <kyrylo.tkachov@arm.com> >> >> PR target/77822 >> * config/aarch64/aarch64.md (*tb<optab><mode>1): Use >> aarch64_simd_shift_imm_<mode> predicate for operand 1. >> (<optab>, ANY_EXTRACT): Use tighter predicates on operands 2 and 3 >> to restrict them to an appropriate range and add FAIL check if the >> region they specify is out of range. Delete useless constraint >> strings. >> (*<optab><mode>, ANY_EXTRACT): Add appropriate predicates on operands >> 2 and 3 to restrict their range and add pattern predicate. >> >> 2016-10-04 Kyrylo Tkachov <kyrylo.tkachov@arm.com> >> >> PR target/77822 >> * g++.dg/torture/pr77822.C: New test. >> >> Kyrill >> >>> >>> On 17/10/16 17:15, Kyrill Tkachov wrote: >>>> Hi all, >>>> >>>> For the attached testcase the code ends up trying to extract bits outside the range of the normal register >>>> widths. The aarch64 patterns for ubfz and tbnz end up accepting such operands and emitting invalid assembly >>>> such as 'ubfx x18,x2,192,32' >>>> >>>> The solution is to add proper predicates and guards to the operands of the zero_extract operations that are going on. >>>> I had a look at all the other patterns in aarch64 that generate/use zero_extract and they all have guards on their >>>> operands in one form or another to avoid them accessing an area that is out of range. >>>> >>>> With this patch the testcase compiles and assembles fine. >>>> >>>> Bootstrapped and tested on aarch64-none-linux-gnu. >>>> >>>> Ok for trunk? >>>> >>>> Thanks, >>>> Kyrill >>>> >>>> 2016-10-17 Kyrylo Tkachov <kyrylo.tkachov@arm.com> >>>> >>>> PR target/77822 >>>> * config/aarch64/aarch64.md (*tb<optab><mode>1): Use >>>> aarch64_simd_shift_imm_<mode> predicate for operand 1. >>>> (<optab>, ANY_EXTRACT): Use tighter predicates on operands 2 and 3 >>>> to restrict them to an appropriate range and add FAIL check if the >>>> region they specify is out of range. Delete useless constraint >>>> strings. >>>> (*<optab><mode>, ANY_EXTRACT): Add appropriate predicates on operands >>>> 2 and 3 to restrict their range and add pattern predicate. >>>> >>>> 2016-10-17 Kyrylo Tkachov <kyrylo.tkachov@arm.com> >>>> >>>> PR target/77822 >>> >> >
On Mon, Oct 17, 2016 at 05:15:21PM +0100, Kyrill Tkachov wrote: > Hi all, > > For the attached testcase the code ends up trying to extract bits outside the > range of the normal register widths. The aarch64 patterns for ubfz and tbnz > end up accepting such operands and emitting invalid assembly > such as 'ubfx x18,x2,192,32' > > The solution is to add proper predicates and guards to the operands of the > zero_extract operations that are going on. I had a look at all the other > patterns in aarch64 that generate/use zero_extract and they all have guards > on their > operands in one form or another to avoid them accessing an area that is out > of range. > > With this patch the testcase compiles and assembles fine. > > Bootstrapped and tested on aarch64-none-linux-gnu. > > Ok for trunk? Ok, sorry for the delay on review. Thanks, James > 2016-10-17 Kyrylo Tkachov <kyrylo.tkachov@arm.com> > > PR target/77822 > * config/aarch64/aarch64.md (*tb<optab><mode>1): Use > aarch64_simd_shift_imm_<mode> predicate for operand 1. > (<optab>, ANY_EXTRACT): Use tighter predicates on operands 2 and 3 > to restrict them to an appropriate range and add FAIL check if the > region they specify is out of range. Delete useless constraint > strings. > (*<optab><mode>, ANY_EXTRACT): Add appropriate predicates on operands > 2 and 3 to restrict their range and add pattern predicate. >
diff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/aarch64.md index 50cfdee909aa717afc097c74ab744c36137352ad..83d45931a28fc7b3442aa0a59b9e7315ef46c0e5 100644 --- a/gcc/config/aarch64/aarch64.md +++ b/gcc/config/aarch64/aarch64.md @@ -639,7 +639,8 @@ (define_insn "*tb<optab><mode>1" [(set (pc) (if_then_else (EQL (zero_extract:DI (match_operand:GPI 0 "register_operand" "r") (const_int 1) - (match_operand 1 "const_int_operand" "n")) + (match_operand 1 + "aarch64_simd_shift_imm_<mode>" "n")) (const_int 0)) (label_ref (match_operand 2 "" "")) (pc))) @@ -4301,19 +4302,28 @@ (define_insn "*extend<GPI:mode>_ashr<SHORT:mode>" (define_expand "<optab>" [(set (match_operand:DI 0 "register_operand" "=r") - (ANY_EXTRACT:DI (match_operand:DI 1 "register_operand" "r") - (match_operand 2 "const_int_operand" "n") - (match_operand 3 "const_int_operand" "n")))] - "" + (ANY_EXTRACT:DI (match_operand:DI 1 "register_operand") + (match_operand 2 + "aarch64_simd_shift_imm_offset_di") + (match_operand 3 "aarch64_simd_shift_imm_di")))] "" + { + if (!IN_RANGE (INTVAL (operands[2]) + INTVAL (operands[3]), + 1, GET_MODE_BITSIZE (DImode) - 1)) + FAIL; + } ) + (define_insn "*<optab><mode>" [(set (match_operand:GPI 0 "register_operand" "=r") (ANY_EXTRACT:GPI (match_operand:GPI 1 "register_operand" "r") - (match_operand 2 "const_int_operand" "n") - (match_operand 3 "const_int_operand" "n")))] - "" + (match_operand 2 + "aarch64_simd_shift_imm_offset_<mode>" "n") + (match_operand 3 + "aarch64_simd_shift_imm_<mode>" "n")))] + "IN_RANGE (INTVAL (operands[2]) + INTVAL (operands[3]), + 1, GET_MODE_BITSIZE (<MODE>mode) - 1)" "<su>bfx\\t%<w>0, %<w>1, %3, %2" [(set_attr "type" "bfm")] ) diff --git a/gcc/testsuite/g++.dg/torture/pr77822.C b/gcc/testsuite/g++.dg/torture/pr77822.C new file mode 100644 index 0000000000000000000000000000000000000000..4dc428b63eee981bda04e1faa29bb97e3986dca9 --- /dev/null +++ b/gcc/testsuite/g++.dg/torture/pr77822.C @@ -0,0 +1,30 @@ +// PR target/77822 +// { dg-do compile } + +using UINT8 = char; +using UINT32 = int; +using UINT64 = long; +class A +{ + void m_fn1 (); + struct B + { + UINT32 m_multiplier; + }; + UINT8 m_datawidth; + UINT8 m_subunits; + B m_subunit_infos[]; +}; +int a; +UINT64 b; +void +A::m_fn1 () +{ + int c = 32, d = m_datawidth / c; + for (int e = 0; e < d; e++) + { + UINT32 f = e * 32; + if (b >> f & 1) + m_subunit_infos[m_subunits].m_multiplier = a; + } +}
Hi all, For the attached testcase the code ends up trying to extract bits outside the range of the normal register widths. The aarch64 patterns for ubfz and tbnz end up accepting such operands and emitting invalid assembly such as 'ubfx x18,x2,192,32' The solution is to add proper predicates and guards to the operands of the zero_extract operations that are going on. I had a look at all the other patterns in aarch64 that generate/use zero_extract and they all have guards on their operands in one form or another to avoid them accessing an area that is out of range. With this patch the testcase compiles and assembles fine. Bootstrapped and tested on aarch64-none-linux-gnu. Ok for trunk? Thanks, Kyrill 2016-10-17 Kyrylo Tkachov <kyrylo.tkachov@arm.com> PR target/77822 * config/aarch64/aarch64.md (*tb<optab><mode>1): Use aarch64_simd_shift_imm_<mode> predicate for operand 1. (<optab>, ANY_EXTRACT): Use tighter predicates on operands 2 and 3 to restrict them to an appropriate range and add FAIL check if the region they specify is out of range. Delete useless constraint strings. (*<optab><mode>, ANY_EXTRACT): Add appropriate predicates on operands 2 and 3 to restrict their range and add pattern predicate. 2016-10-17 Kyrylo Tkachov <kyrylo.tkachov@arm.com> PR target/77822