namaskaaram Revised patch is attached. This patch no longer changes the default option value for xl-barrel-shift
On Mon, Jul 14, 2025 at 10:29 AM Gopi Kumar Bulusu <g...@sankhya.com> wrote: > > > On Fri, Jul 11, 2025 at 8:12 PM Michael Eager <ea...@eagercon.com> wrote: > >> On 7/10/25 4:41 AM, Gopi Kumar Bulusu wrote: >> > >> > namaskaaram >> >> Hi Gopi! >> >> > >> > Please find the patch attached. This addresses regression for >> MicroBlaze >> > (PR118280) >> >> Neal Frager posted a different patch (or an RFC) to address pr118280 on >> 7/1/25: >> https://www.mail-archive.com/gcc-patches@gcc.gnu.org/msg376017.html >> >> The patches are different. Is your patch a replacement for Neal's? >> Can you either reconcile the differences or tell me which patch is >> correct or better? >> > > This patch overrides the previous one - it works with/without barrel > shifter. > > >> >> You might also update the PR with this patch and a comment. >> pr118280 https://gcc.gnu.org/bugzilla/show_bug.cgi?id=118280 >> > > Will do. > > >> > Atomic support enhanced to fix existing atomic_compare_and_swapsi >> pattern >> > to handle side effects; new patterns atomic_fetch_op and >> atomic_test_and_set >> > added. As MicroBlaze has no QImode test/set instruction, use shift magic >> > to implement atomic_test_and_set. Make -mxl-barrel-shift the default to >> keep >> > the default atomics code tight. >> >> Include the PR which is resolved in the patch comments. >> > > It is already there (Subject) > Added PR in comments. > > >> >> I'm not quite sure what you mean by "keep the default atomics code tight". >> >> Neal suggested making -mxl-barrel-shift default for linux, but not >> default for bare-metal. This might be better for backward >> compatibility, but depends on whether there are any MB configurations >> which do not include a barrel shifter. If someone does have a MB config >> without a barrel shifter and tries to recompile after this patch, it's >> possible that invalid code would be silently generated. >> > > I will address this and submit a revision-2 patch. > With this patch revision there are no changes to default value of xl-barrel-shift > > dhanyavaadaaha > gopi > > >> >> > >> > Files Changed >> > >> > * gcc/config/microblaze/iterators.md: New >> > * microblaze-protos.h/microblaze.cc : Add microblaze_subword_address >> > * gcc/config/microblaze/microblaze.md: constants: Add UNSPECV_CAS_BOOL, >> > UNSPECV_CAS_MEM, UNSPECV_CAS_VAL, UNSPECV_ATOMIC_FETCH_OP >> > type: add atomic >> > * gcc/config/microblaze/microblaze.h: TARGET_DEFAULT : Add >> MASK_BARREL_SHIFT >> > * gcc/config/microblaze/sync.md: Add atomic_fetch_<atomic_optab>si >> > atomic_test_and_set >> > >> > Target Checked >> > microblazeel-amd-linux >> > >> > Testing >> > >> > deja-g++ >> > >> > === g++ Summary === >> > >> > # of expected passes 237906 >> > # of unexpected failures 4165 >> > # of unexpected successes 3 >> > # of expected failures 2180 >> > # of unresolved testcases 645 >> > # of unsupported tests 2658 >> > >> > >> > deja-libstdcpp >> > >> > === libstdc++ Summary === >> > >> > # of expected passes 18180 >> > # of unexpected failures 311 >> > # of expected failures 133 >> > # of unresolved testcases 18 >> > # of unsupported tests 853 >> > >> > Includes Test case 29_atomics/atomic_flag/clear/1.cc (which checks for >> > atomic_test_and_set) >> >> I don't have a baseline to compare these test results with, or test >> results before applying this patch, so the results don't have any >> meaning to me. Was the test case 29 failing before applying the patch >> and succeeding after? Were there any other differences? >> > The test case did not build and fails execution without the patch. patch-v2 yields exactly the same results for libstdc++ tests as above. > >> Neal indicated that patch was tested using buildroot with >> target=microblazeel-buildroot-linux-gnu. It looks like you have >> target=microblazeel-amd-linux. How are you running the test suite? >> > Using QEMU from peta-linux image. dhanyavaadaaha gopi > >> >> -- >> Michael Eager >> >>
From cd01b277c4a9e03c83427290228e4b289a169431 Mon Sep 17 00:00:00 2001 From: Gopi Kumar Bulusu <g...@sankhya.com> Date: Thu, 10 Jul 2025 12:44:44 +0530 Subject: [PATCH v2] MicroBlaze : Enhance support for atomics. Fix PR118280 Atomic support enhanced to fix existing atomic_compare_and_swapsi pattern to handle side effects; new patterns atomic_fetch_op and atomic_test_and_set added. As MicroBlaze has no QImode test/set instruction, use shift magic to implement atomic_test_and_set. This fixes PR118280. Files Changed * gcc/config/microblaze/iterators.md: New * microblaze-protos.h/microblaze.cc : Add microblaze_subword_address * gcc/config/microblaze/microblaze.md: constants: Add UNSPECV_CAS_BOOL, UNSPECV_CAS_MEM, UNSPECV_CAS_VAL, UNSPECV_ATOMIC_FETCH_OP type: add atomic * gcc/config/microblaze/sync.md: Add atomic_fetch_<atomic_optab>si atomic_test_and_set Signed-off-by: Kirk Meyer <kirk.me...@sencore.com> Signed-off-by: David Holsgrove <david.holsgr...@xilinx.com> Signed-off-by: Gopi Kumar Bulusu <g...@sankhya.com> --- gcc/config/microblaze/iterators.md | 25 +++++ gcc/config/microblaze/microblaze-protos.h | 1 + gcc/config/microblaze/microblaze.cc | 28 ++++++ gcc/config/microblaze/microblaze.h | 2 +- gcc/config/microblaze/microblaze.md | 7 +- gcc/config/microblaze/sync.md | 107 ++++++++++++++++++---- 6 files changed, 150 insertions(+), 20 deletions(-) create mode 100644 gcc/config/microblaze/iterators.md diff --git a/gcc/config/microblaze/iterators.md b/gcc/config/microblaze/iterators.md new file mode 100644 index 00000000000..2ffc2422a0a --- /dev/null +++ b/gcc/config/microblaze/iterators.md @@ -0,0 +1,25 @@ +;; Iterator definitions for GCC MicroBlaze machine description files. +;; Copyright (C) 2012-2024 Free Software Foundation, Inc. +;; +;; This file is part of GCC. +;; +;; GCC is free software; you can redistribute it and/or modify +;; it under the terms of the GNU General Public License as published by +;; the Free Software Foundation; either version 3, or (at your option) +;; any later version. +;; +;; GCC is distributed in the hope that it will be useful, +;; but WITHOUT ANY WARRANTY; without even the implied warranty of +;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +;; GNU General Public License for more details. +;; +;; You should have received a copy of the GNU General Public License +;; along with GCC; see the file COPYING3. If not see +;; <http://www.gnu.org/licenses/>. + +; atomics code iterator +(define_code_iterator any_atomic [plus ior xor and]) + +; atomics code attribute +(define_code_attr atomic_optab + [(plus "add") (ior "or") (xor "xor") (and "and")]) diff --git a/gcc/config/microblaze/microblaze-protos.h b/gcc/config/microblaze/microblaze-protos.h index 90b79cfe716..1cee0b3096c 100644 --- a/gcc/config/microblaze/microblaze-protos.h +++ b/gcc/config/microblaze/microblaze-protos.h @@ -62,6 +62,7 @@ extern int symbol_mentioned_p (rtx); extern int label_mentioned_p (rtx); extern bool microblaze_cannot_force_const_mem (machine_mode, rtx); extern void microblaze_eh_return (rtx op0); +extern void microblaze_subword_address (rtx, rtx *, rtx *); #endif /* RTX_CODE */ /* Declare functions in microblaze-c.cc. */ diff --git a/gcc/config/microblaze/microblaze.cc b/gcc/config/microblaze/microblaze.cc index 2ab5ada4ec9..80d10ab60e0 100644 --- a/gcc/config/microblaze/microblaze.cc +++ b/gcc/config/microblaze/microblaze.cc @@ -1299,6 +1299,34 @@ microblaze_expand_block_move (rtx dest, rtx src, rtx length, rtx align_rtx) return false; } +/* Compute memory address *aligned_mem and corresponding shift value (*shift) + from a QImode memory reference MEM */ +void +microblaze_subword_address (rtx mem, rtx *aligned_mem, rtx *shift) +{ + /* Align the memory address to a word. */ + rtx addr = force_reg (Pmode, XEXP (mem, 0)); + + rtx addr_mask = gen_int_mode (-4, Pmode); + + rtx aligned_addr = gen_reg_rtx (Pmode); + + emit_move_insn (aligned_addr, gen_rtx_AND (Pmode, addr, addr_mask)); + + *aligned_mem = change_address (mem, SImode, aligned_addr); + + /* Calculate the shift amount. */ + emit_move_insn (*shift, gen_rtx_AND (SImode, addr, gen_int_mode (3, SImode))); + + if (TARGET_LITTLE_ENDIAN == 0) { + emit_move_insn (*shift, + gen_rtx_MINUS (SImode, gen_int_mode (3, SImode), *shift)); + } + + emit_move_insn (*shift, gen_rtx_ASHIFT (SImode, *shift, + gen_int_mode (3, SImode))); +} + static bool microblaze_rtx_costs (rtx x, machine_mode mode, int outer_code ATTRIBUTE_UNUSED, int opno ATTRIBUTE_UNUSED, int *total, diff --git a/gcc/config/microblaze/microblaze.h b/gcc/config/microblaze/microblaze.h index 2390542434b..b2e9ccde363 100644 --- a/gcc/config/microblaze/microblaze.h +++ b/gcc/config/microblaze/microblaze.h @@ -57,7 +57,7 @@ extern enum pipeline_type microblaze_pipe; /* Default target_flags if no switches are specified */ #define TARGET_DEFAULT (MASK_SOFT_MUL | MASK_SOFT_DIV | MASK_SOFT_FLOAT \ - | TARGET_ENDIAN_DEFAULT) + | TARGET_ENDIAN_DEFAULT) /* Do we have CLZ? */ #define TARGET_HAS_CLZ (TARGET_PATTERN_COMPARE && microblaze_has_clz) diff --git a/gcc/config/microblaze/microblaze.md b/gcc/config/microblaze/microblaze.md index 45c48a71e8d..270df8da0c5 100644 --- a/gcc/config/microblaze/microblaze.md +++ b/gcc/config/microblaze/microblaze.md @@ -21,6 +21,7 @@ (include "constraints.md") (include "predicates.md") +(include "iterators.md") ;;---------------------------------------------------- ;; Constants @@ -43,6 +44,10 @@ (UNSPEC_TLS 106) ;; jump table (UNSPEC_SET_TEXT 107) ;; set text start (UNSPEC_TEXT 108) ;; data text relative + (UNSPECV_CAS_BOOL 201) ;; compare and swap (bool) + (UNSPECV_CAS_VAL 202) ;; compare and swap (val) + (UNSPECV_CAS_MEM 203) ;; compare and swap (mem) + (UNSPECV_ATOMIC_FETCH_OP 204) ;; atomic fetch op ]) (define_c_enum "unspec" [ @@ -79,7 +84,7 @@ ;; bshift Shift operations (define_attr "type" - "unknown,branch,jump,call,load,store,move,arith,darith,imul,idiv,icmp,multi,nop,no_delay_arith,no_delay_load,no_delay_store,no_delay_imul,no_delay_move,bshift,fadd,frsub,fmul,fdiv,fcmp,fsl,fsqrt,fcvt,trap" + "unknown,branch,jump,call,load,store,move,arith,darith,imul,idiv,icmp,multi,nop,no_delay_arith,no_delay_load,no_delay_store,no_delay_imul,no_delay_move,bshift,fadd,frsub,fmul,fdiv,fcmp,fsl,fsqrt,fcvt,trap,atomic" (const_string "unknown")) ;; Main data type used by the insn diff --git a/gcc/config/microblaze/sync.md b/gcc/config/microblaze/sync.md index db7b11e5379..6478ab6dcac 100644 --- a/gcc/config/microblaze/sync.md +++ b/gcc/config/microblaze/sync.md @@ -18,26 +18,97 @@ ;; <http://www.gnu.org/licenses/>. (define_insn "atomic_compare_and_swapsi" - [(match_operand:SI 0 "register_operand" "=&d") ;; bool output - (match_operand:SI 1 "register_operand" "=&d") ;; val output - (match_operand:SI 2 "nonimmediate_operand" "+Q") ;; memory - (match_operand:SI 3 "register_operand" "d") ;; expected value - (match_operand:SI 4 "register_operand" "d") ;; desired value - (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 + [(set (match_operand:SI 0 "register_operand" "=&d") ;; bool output + (unspec_volatile:SI + [(match_operand:SI 2 "nonimmediate_operand" "+Q") ;; memory + (match_operand:SI 3 "register_operand" "d") ;; expected value + (match_operand:SI 4 "register_operand" "d")] ;; desired value + UNSPECV_CAS_BOOL)) + (set (match_operand:SI 1 "register_operand" "=&d") ;; val output + (unspec_volatile:SI [(const_int 0)] UNSPECV_CAS_VAL)) + (set (match_dup 2) + (unspec_volatile:SI [(const_int 0)] UNSPECV_CAS_MEM)) + (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 (clobber (match_scratch:SI 8 "=&d"))] "" { - output_asm_insn ("addc \tr0,r0,r0", operands); - output_asm_insn ("lwx \t%1,%y2,r0", operands); - output_asm_insn ("addic\t%8,r0,0", operands); - output_asm_insn ("bnei \t%8,.-8", operands); - output_asm_insn ("cmp \t%0,%1,%3", operands); - output_asm_insn ("bnei \t%0,.+16", operands); - output_asm_insn ("swx \t%4,%y2,r0", operands); - output_asm_insn ("addic\t%8,r0,0", operands); - output_asm_insn ("bnei \t%8,.-28", operands); - return ""; + return "add \t%0,r0,r0\n\t" + "lwx \t%1,%y2,r0\n\t" + "addic\t%8,r0,0\n\t" + "bnei \t%8,.-8\n\t" + "cmp \t%8,%1,%3\n\t" + "bnei \t%8,.+20\n\t" + "swx \t%4,%y2,r0\n\t" + "addic\t%8,r0,0\n\t" + "bnei \t%8,.-28\n\t" + "addi \t%0,r0,1"; } + [(set_attr "type" "atomic") + (set_attr "mode" "SI") + (set_attr "length" "40")] ) + +;; +;; +;; +;; +(define_insn "atomic_fetch_<atomic_optab>si" + [(set (match_operand:SI 0 "register_operand" "=&d") + (match_operand:SI 1 "memory_operand" "+Q")) + (set (match_dup 1) + (unspec_volatile:SI + [(any_atomic:SI (match_dup 1) + (match_operand:SI 2 "register_operand" "d")) + (match_operand:SI 3 "const_int_operand")] ;; model + UNSPECV_ATOMIC_FETCH_OP)) + (clobber (match_scratch:SI 4 "=&d"))] ;; tmp_1 + "" + { + return + "lwx \t%0,%y1,r0\n\t" + "addic\t%4,r0,0\n\t" + "bnei \t%4,.-8\n\t" + "<atomic_optab>\t%4,%0,%2\n\t" + "swx \t%4,%y1,r0\n\t" + "addic\t%4,r0,0\n\t" + "bnei \t%4,.-24"; + } + [(set_attr "type" "atomic") + (set_attr "mode" "SI") + (set_attr "length" "28")]) + +;; +;; MicroBlaze only supports lx/sx instructions for word mode only +;; +;; Use shift|mask magic to implement atomic_test_and_set using lwx/swx +;; +(define_expand "atomic_test_and_set" + [(match_operand:QI 0 "register_operand" "") ;; bool output + (match_operand:QI 1 "memory_operand" "m") ;; memory + (match_operand:SI 2 "const_int_operand" "")] ;; model + "" +{ + rtx old = gen_reg_rtx (SImode); + rtx mem = operands[1]; + rtx model = operands[2]; + rtx set = gen_reg_rtx (SImode); + rtx aligned_mem = gen_reg_rtx (SImode); + rtx shift = gen_reg_rtx (SImode); + + microblaze_subword_address (mem, &aligned_mem, &shift); + + emit_move_insn (set, GEN_INT (1)); + rtx shifted_set = gen_reg_rtx (SImode); + + emit_move_insn (shifted_set, gen_rtx_ASHIFT (SImode, set, shift)); + + emit_insn (gen_atomic_fetch_orsi (old, aligned_mem, shifted_set, model)); + + emit_move_insn (old, gen_rtx_ASHIFTRT (SImode, old, shift)); + + emit_move_insn (operands[0], gen_lowpart (QImode, old)); + + DONE; +}) -- 2.43.5