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

Reply via email to