On 4/18/23 09:59, Jeff Law wrote:
On 4/18/23 08:28, Patrick O'Neill wrote:
...
+ rtx addr = force_reg (Pmode, XEXP (mem, 0));
+
+ rtx aligned_addr = gen_reg_rtx (Pmode);
+ emit_move_insn (aligned_addr, gen_rtx_AND (Pmode, addr,
+ gen_int_mode (-4, Pmode)));
So rather than -4 as a magic number, GET_MODE_MASK would be better.
That may result in needing to rewrap this code. I'd bring the
gen_rtx_AND down on a new line, aligned with aligned_addr.
IIUC GET_MODE_MASK generates masks like 0xFF for QI (for example). It
doesn't have the granularity to generate 0x3 (which we can NOT to get
-4). I searched the GCC internals docs but couldn't find a function that
does address alignment masks.
Presumably using SImode is intentional here rather than wanting to use
word_mode which would be SImode for rv32 and DImode for rv64? I'm
going to work based on that assumption, but if it isn't there's more
work to do to generalize this code.
It's been a year but IIRC it was just simpler to implement (and to me it
didn't make sense to use 64 bits for a subword op).
Is there a benefit in using 64 bit instructions when computing subwords?
+
+(define_expand "atomic_fetch_nand<mode>"
+ [(set (match_operand:SHORT 0 "register_operand" "=&r")
+ (match_operand:SHORT 1 "memory_operand" "+A"))
+ (set (match_dup 1)
+ (unspec_volatile:SHORT
+ [(not:SHORT (and:SHORT (match_dup 1)
+ (match_operand:SHORT 2 "reg_or_0_operand" "rJ")))
+ (match_operand:SI 3 "const_int_operand")] ;; model
+ UNSPEC_SYNC_OLD_OP_SUBWORD))]
+ "TARGET_ATOMIC && TARGET_INLINE_SUBWORD_ATOMIC"
Just a note, constraints aren't necessary for a define_expand. They
don't hurt anything though. They do document expectations, but then
you have to maintain them over time. I'm OK leaving them, mostly
wanted to make sure you're aware they aren't strictly necessary for a
define_expand.
I wasn't aware, thanks for pointing it out! - you're referring to the
"TARGET_ATOMIC && TARGET_INLINE_SUBWORD_ATOMIC", (not the register
constraints) right?
...
Thanks for reviewing!
Patrick
