Hi Richard,
A few comments:
+void
+aarch64_expand_stshh_builtin (tree exp, int fcode)
+{
+ machine_mode mode = TYPE_MODE (TREE_TYPE (CALL_EXPR_ARG (exp, 1)));
+ rtx val = expand_normal (CALL_EXPR_ARG (exp, 1));
+ rtx mem_order = expand_normal (CALL_EXPR_ARG (exp, 2));
+ rtx ret = expand_normal (CALL_EXPR_ARG (exp, 3));
We'll need a check mem_order and ret are constants and within range.
+ switch (fcode)
+ {
+ case AARCH64_BUILTIN_STSHH_SF:
+ {
+ rtx ival = gen_reg_rtx (SImode);
+ emit_move_insn (ival, gen_lowpart (SImode, val));
We prefer to use force_lowpart_subreg for these cases, see eg. isfinite
expander.
+ val = ival;
+ mode = SImode;
+ break;
+ }
+ case AARCH64_BUILTIN_STSHH_DF:
+ {
+ rtx ival = gen_reg_rtx (DImode);
+ emit_move_insn (ival, gen_lowpart (DImode, val));
Same here.
+ val = ival;
+ mode = DImode;
+ break;
+ }
+ default:
+ {
+ if (!register_operand (val, mode))
+ val = force_reg (mode, val);
+ break;
+ }
+ }
--- a/gcc/config/aarch64/aarch64-option-extensions.def
+++ b/gcc/config/aarch64/aarch64-option-extensions.def
@@ -289,6 +289,8 @@ AARCH64_OPT_EXTENSION ("sme-lutv2", SME_LUTv2, (SME2), (),
(), "sme-lutv2")
AARCH64_OPT_EXTENSION("cpa", CPA, (), (), (), "")
+AARCH64_OPT_EXTENSION ("pcdphint", PCDPHINT, (), (), (), "")
I think this part was already separately committed on latest trunk.
+++ b/gcc/config/aarch64/arm_acle.h
+#define __atomic_store_with_stshh(addr, value, memory_order, ret) \
+({ \
+ __auto_type ptr = (addr); \
+ typedef __typeof__ (*ptr) ptr_type; \
+ _Generic ((*addr), \
+ char: __builtin_aarch64_stshh_qi, \
+ unsigned char: __builtin_aarch64_stshh_qi, \
+ signed char: __builtin_aarch64_stshh_qi, \
+ unsigned short: __builtin_aarch64_stshh_hi, \
+ short: __builtin_aarch64_stshh_hi, \
+ unsigned int: __builtin_aarch64_stshh_si, \
+ int: __builtin_aarch64_stshh_si, \
+ unsigned long: __builtin_aarch64_stshh_di, \
+ long: __builtin_aarch64_stshh_di, \
+ unsigned long long: __builtin_aarch64_stshh_di, \
+ long long: __builtin_aarch64_stshh_di, \
+ float: __builtin_aarch64_stshh_sf, \
+ double: __builtin_aarch64_stshh_df, \
+ default: __builtin_aarch64_stshh_di \
+ )((addr), (ptr_type)(value), (memory_order), (ret)); \
+})
This works fine now - I still think it is not ideal, but we can always improve
it
in the future.
diff --git a/gcc/config/aarch64/atomics.md b/gcc/config/aarch64/atomics.md
index d4b4afb5815..db9b4fda6fe 100644
--- a/gcc/config/aarch64/atomics.md
+++ b/gcc/config/aarch64/atomics.md
@@ -751,6 +751,29 @@
[(set_attr "arch" "*,rcpc8_4")]
)
+(define_insn "@aarch64_atomic_store_stshh<mode>"
+ [(set (match_operand:ALLI 0 "aarch64_sync_memory_operand" "=Q,Ust")
Surely aarch64_rcpc_memory_operand like atomic_store above?
+ (unspec_volatile:ALLI
+ [(match_operand:ALLI 1 "register_operand" "r,r")
Surely rZ,rZ? And it would be nice to have zero stored in one of the test cases.
+ (match_operand:SI 2 "const_int_operand") ;; model
+ (match_operand:SI 3 "const_int_operand")] ;; ret_policy
+ UNSPECV_STSHH))]
+ ""
+ {
+ if (INTVAL (operands[3]) == 0)
+ output_asm_insn ("stshh keep", operands);
Use '\t' rather than a space before 'keep' / 'strm'.
+ else
+ output_asm_insn ("stshh strm", operands);
+ enum memmodel model = memmodel_from_int (INTVAL (operands[2]));
+ if (is_mm_relaxed (model))
This will also need || is_mm_consume (model) || is_mm_acquire (model) unless
you explicitly block those in the intrinsic expansion.
+ return "str<atomic_sfx>\t%<w>1, %0";
+ else if (which_alternative == 0)
+ return "stlr<atomic_sfx>\t%<w>1, %0";
+ else
+ return "stlur<atomic_sfx>\t%<w>1, %0";
+ }
Missing [(set_attr "arch" "*,rcpc8_4")]
diff --git a/gcc/testsuite/gcc.target/aarch64/atomic_store_with_stshh.c.c
b/gcc/testsuite/gcc.target/aarch64/atomic_store_with_stshh.c.c
Can we just use atomic_store_with_stshh.c rather than .c.c?
Can we have a few cases with a non-zero pointer offset and a store of zero to
verify those cases
work correctly?
\ No newline at end of file
Please fix.
Cheers,
Wilco
