| Message ID | CAAgBjMnMqXAnbtnTT4Q_0GDFw9X3mRfbActR1bMkU73Cg_x6Yg@mail.gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | PR90724 - ICE with __sync_bool_compare_and_swap with -march=armv8.2-a | expand |
On Wed, 10 Jul 2019 at 16:54, Prathamesh Kulkarni <prathamesh.kulkarni@linaro.org> wrote: > > Hi, > For following test-case, > static long long AL[24]; > > int > check_ok (void) > { > return (__sync_bool_compare_and_swap (AL+1, 0x200000003ll, 0x1234567890ll)); > } > > Compiling with -O2 -march=armv8.2-a results in: > pr90724.c: In function ‘check_ok’: > pr90724.c:7:1: error: unrecognizable insn: > 7 | } > | ^ > (insn 11 10 12 2 (set (reg:CC 66 cc) > (compare:CC (reg:DI 95) > (const_int 8589934595 [0x200000003]))) "pr90724.c":6:11 -1 > (nil)) > > IIUC, the issue is that 0x200000003 falls outside the range of > allowable immediate in cmp ? If it's replaced by a small constant then it works. > > The ICE results with -march=armv8.2-a because, we enter if > (TARGET_LSE) { ... } condition > in aarch64_expand_compare_and_swap, while with -march=armv8.a it goes into else, > which forces oldval into register if the predicate fails to match. > > The attached patch checks if y (oldval) satisfies aarch64_plus_operand > predicate and if not, forces it to be in register, which resolves ICE. > Does it look OK ? > > Bootstrap+testing in progress on aarch64-linux-gnu. ping https://gcc.gnu.org/ml/gcc-patches/2019-07/msg00793.html Thanks, Prathamesh > > PS: The issue has nothing to do with SVE, which I incorrectly > mentioned in bug report. > > Thanks, > Prathamesh
Hi Prathamesh On 7/10/19 12:24 PM, Prathamesh Kulkarni wrote: > Hi, > For following test-case, > static long long AL[24]; > > int > check_ok (void) > { > return (__sync_bool_compare_and_swap (AL+1, 0x200000003ll, > 0x1234567890ll)); > } > > Compiling with -O2 -march=armv8.2-a results in: > pr90724.c: In function ‘check_ok’: > pr90724.c:7:1: error: unrecognizable insn: > 7 | } > | ^ > (insn 11 10 12 2 (set (reg:CC 66 cc) > (compare:CC (reg:DI 95) > (const_int 8589934595 [0x200000003]))) "pr90724.c":6:11 -1 > (nil)) > > IIUC, the issue is that 0x200000003 falls outside the range of > allowable immediate in cmp ? If it's replaced by a small constant then > it works. > > The ICE results with -march=armv8.2-a because, we enter if > (TARGET_LSE) { ... } condition > in aarch64_expand_compare_and_swap, while with -march=armv8.a it goes > into else, > which forces oldval into register if the predicate fails to match. > > The attached patch checks if y (oldval) satisfies aarch64_plus_operand > predicate and if not, forces it to be in register, which resolves ICE. > Does it look OK ? > > Bootstrap+testing in progress on aarch64-linux-gnu. > > PS: The issue has nothing to do with SVE, which I incorrectly > mentioned in bug report. > This looks ok to me (but you'll need maintainer approval). Does this fail on the branches as well? Thanks, Kyrill > Thanks, > Prathamesh
On Wed, 17 Jul 2019 at 13:45, Kyrill Tkachov <kyrylo.tkachov@foss.arm.com> wrote: > > Hi Prathamesh > > On 7/10/19 12:24 PM, Prathamesh Kulkarni wrote: > > Hi, > > For following test-case, > > static long long AL[24]; > > > > int > > check_ok (void) > > { > > return (__sync_bool_compare_and_swap (AL+1, 0x200000003ll, > > 0x1234567890ll)); > > } > > > > Compiling with -O2 -march=armv8.2-a results in: > > pr90724.c: In function ‘check_ok’: > > pr90724.c:7:1: error: unrecognizable insn: > > 7 | } > > | ^ > > (insn 11 10 12 2 (set (reg:CC 66 cc) > > (compare:CC (reg:DI 95) > > (const_int 8589934595 [0x200000003]))) "pr90724.c":6:11 -1 > > (nil)) > > > > IIUC, the issue is that 0x200000003 falls outside the range of > > allowable immediate in cmp ? If it's replaced by a small constant then > > it works. > > > > The ICE results with -march=armv8.2-a because, we enter if > > (TARGET_LSE) { ... } condition > > in aarch64_expand_compare_and_swap, while with -march=armv8.a it goes > > into else, > > which forces oldval into register if the predicate fails to match. > > > > The attached patch checks if y (oldval) satisfies aarch64_plus_operand > > predicate and if not, forces it to be in register, which resolves ICE. > > Does it look OK ? > > > > Bootstrap+testing in progress on aarch64-linux-gnu. > > > > PS: The issue has nothing to do with SVE, which I incorrectly > > mentioned in bug report. > > > This looks ok to me (but you'll need maintainer approval). > > Does this fail on the branches as well? Hi Kyrill, Thanks for the review. The test also fails on gcc-9-branch (but not on gcc-8). Thanks, Prathamesh > > Thanks, > > Kyrill > > > > Thanks, > > Prathamesh
On Wed, 17 Jul 2019 at 18:15, Prathamesh Kulkarni <prathamesh.kulkarni@linaro.org> wrote: > > On Wed, 17 Jul 2019 at 13:45, Kyrill Tkachov > <kyrylo.tkachov@foss.arm.com> wrote: > > > > Hi Prathamesh > > > > On 7/10/19 12:24 PM, Prathamesh Kulkarni wrote: > > > Hi, > > > For following test-case, > > > static long long AL[24]; > > > > > > int > > > check_ok (void) > > > { > > > return (__sync_bool_compare_and_swap (AL+1, 0x200000003ll, > > > 0x1234567890ll)); > > > } > > > > > > Compiling with -O2 -march=armv8.2-a results in: > > > pr90724.c: In function ‘check_ok’: > > > pr90724.c:7:1: error: unrecognizable insn: > > > 7 | } > > > | ^ > > > (insn 11 10 12 2 (set (reg:CC 66 cc) > > > (compare:CC (reg:DI 95) > > > (const_int 8589934595 [0x200000003]))) "pr90724.c":6:11 -1 > > > (nil)) > > > > > > IIUC, the issue is that 0x200000003 falls outside the range of > > > allowable immediate in cmp ? If it's replaced by a small constant then > > > it works. > > > > > > The ICE results with -march=armv8.2-a because, we enter if > > > (TARGET_LSE) { ... } condition > > > in aarch64_expand_compare_and_swap, while with -march=armv8.a it goes > > > into else, > > > which forces oldval into register if the predicate fails to match. > > > > > > The attached patch checks if y (oldval) satisfies aarch64_plus_operand > > > predicate and if not, forces it to be in register, which resolves ICE. > > > Does it look OK ? > > > > > > Bootstrap+testing in progress on aarch64-linux-gnu. > > > > > > PS: The issue has nothing to do with SVE, which I incorrectly > > > mentioned in bug report. > > > > > This looks ok to me (but you'll need maintainer approval). > > > > Does this fail on the branches as well? > Hi Kyrill, > Thanks for the review. The test also fails on gcc-9-branch (but not on gcc-8). Hi James, Is the patch OK to commit ? https://gcc.gnu.org/ml/gcc-patches/2019-07/msg00793.html Thanks, Prathamesh > > Thanks, > Prathamesh > > > > Thanks, > > > > Kyrill > > > > > > > Thanks, > > > Prathamesh
On Thu, 25 Jul 2019 at 11:56, Prathamesh Kulkarni <prathamesh.kulkarni@linaro.org> wrote: > > On Wed, 17 Jul 2019 at 18:15, Prathamesh Kulkarni > <prathamesh.kulkarni@linaro.org> wrote: > > > > On Wed, 17 Jul 2019 at 13:45, Kyrill Tkachov > > <kyrylo.tkachov@foss.arm.com> wrote: > > > > > > Hi Prathamesh > > > > > > On 7/10/19 12:24 PM, Prathamesh Kulkarni wrote: > > > > Hi, > > > > For following test-case, > > > > static long long AL[24]; > > > > > > > > int > > > > check_ok (void) > > > > { > > > > return (__sync_bool_compare_and_swap (AL+1, 0x200000003ll, > > > > 0x1234567890ll)); > > > > } > > > > > > > > Compiling with -O2 -march=armv8.2-a results in: > > > > pr90724.c: In function ‘check_ok’: > > > > pr90724.c:7:1: error: unrecognizable insn: > > > > 7 | } > > > > | ^ > > > > (insn 11 10 12 2 (set (reg:CC 66 cc) > > > > (compare:CC (reg:DI 95) > > > > (const_int 8589934595 [0x200000003]))) "pr90724.c":6:11 -1 > > > > (nil)) > > > > > > > > IIUC, the issue is that 0x200000003 falls outside the range of > > > > allowable immediate in cmp ? If it's replaced by a small constant then > > > > it works. > > > > > > > > The ICE results with -march=armv8.2-a because, we enter if > > > > (TARGET_LSE) { ... } condition > > > > in aarch64_expand_compare_and_swap, while with -march=armv8.a it goes > > > > into else, > > > > which forces oldval into register if the predicate fails to match. > > > > > > > > The attached patch checks if y (oldval) satisfies aarch64_plus_operand > > > > predicate and if not, forces it to be in register, which resolves ICE. > > > > Does it look OK ? > > > > > > > > Bootstrap+testing in progress on aarch64-linux-gnu. > > > > > > > > PS: The issue has nothing to do with SVE, which I incorrectly > > > > mentioned in bug report. > > > > > > > This looks ok to me (but you'll need maintainer approval). > > > > > > Does this fail on the branches as well? > > Hi Kyrill, > > Thanks for the review. The test also fails on gcc-9-branch (but not on gcc-8). > Hi James, > Is the patch OK to commit ? > https://gcc.gnu.org/ml/gcc-patches/2019-07/msg00793.html ping * 3: https://gcc.gnu.org/ml/gcc-patches/2019-07/msg00793.html Thanks, Prathamesh > > Thanks, > Prathamesh > > > > Thanks, > > Prathamesh > > > > > > Thanks, > > > > > > Kyrill > > > > > > > > > > Thanks, > > > > Prathamesh
On Thu, 1 Aug 2019 at 15:34, Prathamesh Kulkarni <prathamesh.kulkarni@linaro.org> wrote: > > On Thu, 25 Jul 2019 at 11:56, Prathamesh Kulkarni > <prathamesh.kulkarni@linaro.org> wrote: > > > > On Wed, 17 Jul 2019 at 18:15, Prathamesh Kulkarni > > <prathamesh.kulkarni@linaro.org> wrote: > > > > > > On Wed, 17 Jul 2019 at 13:45, Kyrill Tkachov > > > <kyrylo.tkachov@foss.arm.com> wrote: > > > > > > > > Hi Prathamesh > > > > > > > > On 7/10/19 12:24 PM, Prathamesh Kulkarni wrote: > > > > > Hi, > > > > > For following test-case, > > > > > static long long AL[24]; > > > > > > > > > > int > > > > > check_ok (void) > > > > > { > > > > > return (__sync_bool_compare_and_swap (AL+1, 0x200000003ll, > > > > > 0x1234567890ll)); > > > > > } > > > > > > > > > > Compiling with -O2 -march=armv8.2-a results in: > > > > > pr90724.c: In function ‘check_ok’: > > > > > pr90724.c:7:1: error: unrecognizable insn: > > > > > 7 | } > > > > > | ^ > > > > > (insn 11 10 12 2 (set (reg:CC 66 cc) > > > > > (compare:CC (reg:DI 95) > > > > > (const_int 8589934595 [0x200000003]))) "pr90724.c":6:11 -1 > > > > > (nil)) > > > > > > > > > > IIUC, the issue is that 0x200000003 falls outside the range of > > > > > allowable immediate in cmp ? If it's replaced by a small constant then > > > > > it works. > > > > > > > > > > The ICE results with -march=armv8.2-a because, we enter if > > > > > (TARGET_LSE) { ... } condition > > > > > in aarch64_expand_compare_and_swap, while with -march=armv8.a it goes > > > > > into else, > > > > > which forces oldval into register if the predicate fails to match. > > > > > > > > > > The attached patch checks if y (oldval) satisfies aarch64_plus_operand > > > > > predicate and if not, forces it to be in register, which resolves ICE. > > > > > Does it look OK ? > > > > > > > > > > Bootstrap+testing in progress on aarch64-linux-gnu. > > > > > > > > > > PS: The issue has nothing to do with SVE, which I incorrectly > > > > > mentioned in bug report. > > > > > > > > > This looks ok to me (but you'll need maintainer approval). > > > > > > > > Does this fail on the branches as well? > > > Hi Kyrill, > > > Thanks for the review. The test also fails on gcc-9-branch (but not on gcc-8). > > Hi James, > > Is the patch OK to commit ? > > https://gcc.gnu.org/ml/gcc-patches/2019-07/msg00793.html > ping * 3: https://gcc.gnu.org/ml/gcc-patches/2019-07/msg00793.html ping * 4: https://gcc.gnu.org/ml/gcc-patches/2019-07/msg00793.html Thanks, Prathamesh > > Thanks, > Prathamesh > > > > Thanks, > > Prathamesh > > > > > > Thanks, > > > Prathamesh > > > > > > > > Thanks, > > > > > > > > Kyrill > > > > > > > > > > > > > Thanks, > > > > > Prathamesh
On Thu, 8 Aug 2019 at 11:22, Prathamesh Kulkarni <prathamesh.kulkarni@linaro.org> wrote: > > On Thu, 1 Aug 2019 at 15:34, Prathamesh Kulkarni > <prathamesh.kulkarni@linaro.org> wrote: > > > > On Thu, 25 Jul 2019 at 11:56, Prathamesh Kulkarni > > <prathamesh.kulkarni@linaro.org> wrote: > > > > > > On Wed, 17 Jul 2019 at 18:15, Prathamesh Kulkarni > > > <prathamesh.kulkarni@linaro.org> wrote: > > > > > > > > On Wed, 17 Jul 2019 at 13:45, Kyrill Tkachov > > > > <kyrylo.tkachov@foss.arm.com> wrote: > > > > > > > > > > Hi Prathamesh > > > > > > > > > > On 7/10/19 12:24 PM, Prathamesh Kulkarni wrote: > > > > > > Hi, > > > > > > For following test-case, > > > > > > static long long AL[24]; > > > > > > > > > > > > int > > > > > > check_ok (void) > > > > > > { > > > > > > return (__sync_bool_compare_and_swap (AL+1, 0x200000003ll, > > > > > > 0x1234567890ll)); > > > > > > } > > > > > > > > > > > > Compiling with -O2 -march=armv8.2-a results in: > > > > > > pr90724.c: In function ‘check_ok’: > > > > > > pr90724.c:7:1: error: unrecognizable insn: > > > > > > 7 | } > > > > > > | ^ > > > > > > (insn 11 10 12 2 (set (reg:CC 66 cc) > > > > > > (compare:CC (reg:DI 95) > > > > > > (const_int 8589934595 [0x200000003]))) "pr90724.c":6:11 -1 > > > > > > (nil)) > > > > > > > > > > > > IIUC, the issue is that 0x200000003 falls outside the range of > > > > > > allowable immediate in cmp ? If it's replaced by a small constant then > > > > > > it works. > > > > > > > > > > > > The ICE results with -march=armv8.2-a because, we enter if > > > > > > (TARGET_LSE) { ... } condition > > > > > > in aarch64_expand_compare_and_swap, while with -march=armv8.a it goes > > > > > > into else, > > > > > > which forces oldval into register if the predicate fails to match. > > > > > > > > > > > > The attached patch checks if y (oldval) satisfies aarch64_plus_operand > > > > > > predicate and if not, forces it to be in register, which resolves ICE. > > > > > > Does it look OK ? > > > > > > > > > > > > Bootstrap+testing in progress on aarch64-linux-gnu. > > > > > > > > > > > > PS: The issue has nothing to do with SVE, which I incorrectly > > > > > > mentioned in bug report. > > > > > > > > > > > This looks ok to me (but you'll need maintainer approval). > > > > > > > > > > Does this fail on the branches as well? > > > > Hi Kyrill, > > > > Thanks for the review. The test also fails on gcc-9-branch (but not on gcc-8). > > > Hi James, > > > Is the patch OK to commit ? > > > https://gcc.gnu.org/ml/gcc-patches/2019-07/msg00793.html > > ping * 3: https://gcc.gnu.org/ml/gcc-patches/2019-07/msg00793.html > ping * 4: https://gcc.gnu.org/ml/gcc-patches/2019-07/msg00793.html ping * 5: https://gcc.gnu.org/ml/gcc-patches/2019-07/msg00793.html Thanks, Prathamesh > > Thanks, > Prathamesh > > > > Thanks, > > Prathamesh > > > > > > Thanks, > > > Prathamesh > > > > > > > > Thanks, > > > > Prathamesh > > > > > > > > > > Thanks, > > > > > > > > > > Kyrill > > > > > > > > > > > > > > > > Thanks, > > > > > > Prathamesh
On Thu, Aug 15, 2019 at 02:11:25PM +0100, Prathamesh Kulkarni wrote: > On Thu, 8 Aug 2019 at 11:22, Prathamesh Kulkarni > <prathamesh.kulkarni@linaro.org> wrote: > > > > On Thu, 1 Aug 2019 at 15:34, Prathamesh Kulkarni > > <prathamesh.kulkarni@linaro.org> wrote: > > > > > > On Thu, 25 Jul 2019 at 11:56, Prathamesh Kulkarni > > > <prathamesh.kulkarni@linaro.org> wrote: > > > > > > > > On Wed, 17 Jul 2019 at 18:15, Prathamesh Kulkarni > > > > <prathamesh.kulkarni@linaro.org> wrote: > > > > > > > > > > On Wed, 17 Jul 2019 at 13:45, Kyrill Tkachov > > > > > <kyrylo.tkachov@foss.arm.com> wrote: > > > > > > > > > > > > Hi Prathamesh > > > > > > > > > > > > On 7/10/19 12:24 PM, Prathamesh Kulkarni wrote: > > > > > > > Hi, > > > > > > > For following test-case, > > > > > > > static long long AL[24]; > > > > > > > > > > > > > > int > > > > > > > check_ok (void) > > > > > > > { > > > > > > > return (__sync_bool_compare_and_swap (AL+1, 0x200000003ll, > > > > > > > 0x1234567890ll)); > > > > > > > } > > > > > > > > > > > > > > Compiling with -O2 -march=armv8.2-a results in: > > > > > > > pr90724.c: In function ‘check_ok’: > > > > > > > pr90724.c:7:1: error: unrecognizable insn: > > > > > > > 7 | } > > > > > > > | ^ > > > > > > > (insn 11 10 12 2 (set (reg:CC 66 cc) > > > > > > > (compare:CC (reg:DI 95) > > > > > > > (const_int 8589934595 [0x200000003]))) "pr90724.c":6:11 -1 > > > > > > > (nil)) > > > > > > > > > > > > > > IIUC, the issue is that 0x200000003 falls outside the range of > > > > > > > allowable immediate in cmp ? If it's replaced by a small constant then > > > > > > > it works. > > > > > > > > > > > > > > The ICE results with -march=armv8.2-a because, we enter if > > > > > > > (TARGET_LSE) { ... } condition > > > > > > > in aarch64_expand_compare_and_swap, while with -march=armv8.a it goes > > > > > > > into else, > > > > > > > which forces oldval into register if the predicate fails to match. > > > > > > > > > > > > > > The attached patch checks if y (oldval) satisfies aarch64_plus_operand > > > > > > > predicate and if not, forces it to be in register, which resolves ICE. > > > > > > > Does it look OK ? > > > > > > > > > > > > > > Bootstrap+testing in progress on aarch64-linux-gnu. > > > > > > > > > > > > > > PS: The issue has nothing to do with SVE, which I incorrectly > > > > > > > mentioned in bug report. > > > > > > > > > > > > > This looks ok to me (but you'll need maintainer approval). > > > > > > > > > > > > Does this fail on the branches as well? > > > > > Hi Kyrill, > > > > > Thanks for the review. The test also fails on gcc-9-branch (but not on gcc-8). > > > > Hi James, > > > > Is the patch OK to commit ? > > > > https://gcc.gnu.org/ml/gcc-patches/2019-07/msg00793.html > > > ping * 3: https://gcc.gnu.org/ml/gcc-patches/2019-07/msg00793.html > > ping * 4: https://gcc.gnu.org/ml/gcc-patches/2019-07/msg00793.html > ping * 5: https://gcc.gnu.org/ml/gcc-patches/2019-07/msg00793.html Hi, Sorry, this missed my filters as it didn't mention AArch64 in the subject line. Thais is good for trunk, thanks for waiting. James
On Mon, 19 Aug 2019 at 22:14, James Greenhalgh <james.greenhalgh@arm.com> wrote: > > On Thu, Aug 15, 2019 at 02:11:25PM +0100, Prathamesh Kulkarni wrote: > > On Thu, 8 Aug 2019 at 11:22, Prathamesh Kulkarni > > <prathamesh.kulkarni@linaro.org> wrote: > > > > > > On Thu, 1 Aug 2019 at 15:34, Prathamesh Kulkarni > > > <prathamesh.kulkarni@linaro.org> wrote: > > > > > > > > On Thu, 25 Jul 2019 at 11:56, Prathamesh Kulkarni > > > > <prathamesh.kulkarni@linaro.org> wrote: > > > > > > > > > > On Wed, 17 Jul 2019 at 18:15, Prathamesh Kulkarni > > > > > <prathamesh.kulkarni@linaro.org> wrote: > > > > > > > > > > > > On Wed, 17 Jul 2019 at 13:45, Kyrill Tkachov > > > > > > <kyrylo.tkachov@foss.arm.com> wrote: > > > > > > > > > > > > > > Hi Prathamesh > > > > > > > > > > > > > > On 7/10/19 12:24 PM, Prathamesh Kulkarni wrote: > > > > > > > > Hi, > > > > > > > > For following test-case, > > > > > > > > static long long AL[24]; > > > > > > > > > > > > > > > > int > > > > > > > > check_ok (void) > > > > > > > > { > > > > > > > > return (__sync_bool_compare_and_swap (AL+1, 0x200000003ll, > > > > > > > > 0x1234567890ll)); > > > > > > > > } > > > > > > > > > > > > > > > > Compiling with -O2 -march=armv8.2-a results in: > > > > > > > > pr90724.c: In function ‘check_ok’: > > > > > > > > pr90724.c:7:1: error: unrecognizable insn: > > > > > > > > 7 | } > > > > > > > > | ^ > > > > > > > > (insn 11 10 12 2 (set (reg:CC 66 cc) > > > > > > > > (compare:CC (reg:DI 95) > > > > > > > > (const_int 8589934595 [0x200000003]))) "pr90724.c":6:11 -1 > > > > > > > > (nil)) > > > > > > > > > > > > > > > > IIUC, the issue is that 0x200000003 falls outside the range of > > > > > > > > allowable immediate in cmp ? If it's replaced by a small constant then > > > > > > > > it works. > > > > > > > > > > > > > > > > The ICE results with -march=armv8.2-a because, we enter if > > > > > > > > (TARGET_LSE) { ... } condition > > > > > > > > in aarch64_expand_compare_and_swap, while with -march=armv8.a it goes > > > > > > > > into else, > > > > > > > > which forces oldval into register if the predicate fails to match. > > > > > > > > > > > > > > > > The attached patch checks if y (oldval) satisfies aarch64_plus_operand > > > > > > > > predicate and if not, forces it to be in register, which resolves ICE. > > > > > > > > Does it look OK ? > > > > > > > > > > > > > > > > Bootstrap+testing in progress on aarch64-linux-gnu. > > > > > > > > > > > > > > > > PS: The issue has nothing to do with SVE, which I incorrectly > > > > > > > > mentioned in bug report. > > > > > > > > > > > > > > > This looks ok to me (but you'll need maintainer approval). > > > > > > > > > > > > > > Does this fail on the branches as well? > > > > > > Hi Kyrill, > > > > > > Thanks for the review. The test also fails on gcc-9-branch (but not on gcc-8). > > > > > Hi James, > > > > > Is the patch OK to commit ? > > > > > https://gcc.gnu.org/ml/gcc-patches/2019-07/msg00793.html > > > > ping * 3: https://gcc.gnu.org/ml/gcc-patches/2019-07/msg00793.html > > > ping * 4: https://gcc.gnu.org/ml/gcc-patches/2019-07/msg00793.html > > ping * 5: https://gcc.gnu.org/ml/gcc-patches/2019-07/msg00793.html > > Hi, > > Sorry, this missed my filters as it didn't mention AArch64 in the subject > line. > > Thais is good for trunk, thanks for waiting. Thanks, committed to trunk in r274805. Is this OK to backport to gcc-9-branch ? Thanks, Prathamesh > > James >
On Thu, 22 Aug 2019 at 00:05, Prathamesh Kulkarni <prathamesh.kulkarni@linaro.org> wrote: > > On Mon, 19 Aug 2019 at 22:14, James Greenhalgh <james.greenhalgh@arm.com> wrote: > > > > On Thu, Aug 15, 2019 at 02:11:25PM +0100, Prathamesh Kulkarni wrote: > > > On Thu, 8 Aug 2019 at 11:22, Prathamesh Kulkarni > > > <prathamesh.kulkarni@linaro.org> wrote: > > > > > > > > On Thu, 1 Aug 2019 at 15:34, Prathamesh Kulkarni > > > > <prathamesh.kulkarni@linaro.org> wrote: > > > > > > > > > > On Thu, 25 Jul 2019 at 11:56, Prathamesh Kulkarni > > > > > <prathamesh.kulkarni@linaro.org> wrote: > > > > > > > > > > > > On Wed, 17 Jul 2019 at 18:15, Prathamesh Kulkarni > > > > > > <prathamesh.kulkarni@linaro.org> wrote: > > > > > > > > > > > > > > On Wed, 17 Jul 2019 at 13:45, Kyrill Tkachov > > > > > > > <kyrylo.tkachov@foss.arm.com> wrote: > > > > > > > > > > > > > > > > Hi Prathamesh > > > > > > > > > > > > > > > > On 7/10/19 12:24 PM, Prathamesh Kulkarni wrote: > > > > > > > > > Hi, > > > > > > > > > For following test-case, > > > > > > > > > static long long AL[24]; > > > > > > > > > > > > > > > > > > int > > > > > > > > > check_ok (void) > > > > > > > > > { > > > > > > > > > return (__sync_bool_compare_and_swap (AL+1, 0x200000003ll, > > > > > > > > > 0x1234567890ll)); > > > > > > > > > } > > > > > > > > > > > > > > > > > > Compiling with -O2 -march=armv8.2-a results in: > > > > > > > > > pr90724.c: In function ‘check_ok’: > > > > > > > > > pr90724.c:7:1: error: unrecognizable insn: > > > > > > > > > 7 | } > > > > > > > > > | ^ > > > > > > > > > (insn 11 10 12 2 (set (reg:CC 66 cc) > > > > > > > > > (compare:CC (reg:DI 95) > > > > > > > > > (const_int 8589934595 [0x200000003]))) "pr90724.c":6:11 -1 > > > > > > > > > (nil)) > > > > > > > > > > > > > > > > > > IIUC, the issue is that 0x200000003 falls outside the range of > > > > > > > > > allowable immediate in cmp ? If it's replaced by a small constant then > > > > > > > > > it works. > > > > > > > > > > > > > > > > > > The ICE results with -march=armv8.2-a because, we enter if > > > > > > > > > (TARGET_LSE) { ... } condition > > > > > > > > > in aarch64_expand_compare_and_swap, while with -march=armv8.a it goes > > > > > > > > > into else, > > > > > > > > > which forces oldval into register if the predicate fails to match. > > > > > > > > > > > > > > > > > > The attached patch checks if y (oldval) satisfies aarch64_plus_operand > > > > > > > > > predicate and if not, forces it to be in register, which resolves ICE. > > > > > > > > > Does it look OK ? > > > > > > > > > > > > > > > > > > Bootstrap+testing in progress on aarch64-linux-gnu. > > > > > > > > > > > > > > > > > > PS: The issue has nothing to do with SVE, which I incorrectly > > > > > > > > > mentioned in bug report. > > > > > > > > > > > > > > > > > This looks ok to me (but you'll need maintainer approval). > > > > > > > > > > > > > > > > Does this fail on the branches as well? > > > > > > > Hi Kyrill, > > > > > > > Thanks for the review. The test also fails on gcc-9-branch (but not on gcc-8). > > > > > > Hi James, > > > > > > Is the patch OK to commit ? > > > > > > https://gcc.gnu.org/ml/gcc-patches/2019-07/msg00793.html > > > > > ping * 3: https://gcc.gnu.org/ml/gcc-patches/2019-07/msg00793.html > > > > ping * 4: https://gcc.gnu.org/ml/gcc-patches/2019-07/msg00793.html > > > ping * 5: https://gcc.gnu.org/ml/gcc-patches/2019-07/msg00793.html > > > > Hi, > > > > Sorry, this missed my filters as it didn't mention AArch64 in the subject > > line. > > > > Thais is good for trunk, thanks for waiting. > Thanks, committed to trunk in r274805. > Is this OK to backport to gcc-9-branch ? ping ? Would it be OK to backport this patch to gcc-9-branch ? Thanks, Prathamesh > > Thanks, > Prathamesh > > > > James > >
diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c index a18fbd0f0aa..22d4726e19a 100644 --- a/gcc/config/aarch64/aarch64.c +++ b/gcc/config/aarch64/aarch64.c @@ -1930,6 +1930,9 @@ aarch64_gen_compare_reg_maybe_ze (RTX_CODE code, rtx x, rtx y, } } + if (!aarch64_plus_operand (y, y_mode)) + y = force_reg (y_mode, y); + return aarch64_gen_compare_reg (code, x, y); }
Hi, For following test-case, static long long AL[24]; int check_ok (void) { return (__sync_bool_compare_and_swap (AL+1, 0x200000003ll, 0x1234567890ll)); } Compiling with -O2 -march=armv8.2-a results in: pr90724.c: In function ‘check_ok’: pr90724.c:7:1: error: unrecognizable insn: 7 | } | ^ (insn 11 10 12 2 (set (reg:CC 66 cc) (compare:CC (reg:DI 95) (const_int 8589934595 [0x200000003]))) "pr90724.c":6:11 -1 (nil)) IIUC, the issue is that 0x200000003 falls outside the range of allowable immediate in cmp ? If it's replaced by a small constant then it works. The ICE results with -march=armv8.2-a because, we enter if (TARGET_LSE) { ... } condition in aarch64_expand_compare_and_swap, while with -march=armv8.a it goes into else, which forces oldval into register if the predicate fails to match. The attached patch checks if y (oldval) satisfies aarch64_plus_operand predicate and if not, forces it to be in register, which resolves ICE. Does it look OK ? Bootstrap+testing in progress on aarch64-linux-gnu. PS: The issue has nothing to do with SVE, which I incorrectly mentioned in bug report. Thanks, Prathamesh 2019-07-10 Prathamesh Kulkarni <prathamesh.kulkarni@linaro.org> PR target/90724 * config/aarch64/aarch64.c (aarch64_gen_compare_reg_maybe_ze): Force y in reg if it fails aarch64_plus_operand predicate.