On 10/22/23 21:55, Tsukasa OI wrote:
What I still don't understand is why we're dealing with a decomposed
address in the builtin, define_expand and/or define_insn.
Sorry, I misunderstood your intent (quite badly) possibly because I was
not familiar with the concept of "predicates" in GCC.
OK. So you might want to read the machine description part of the GCC
manual. It describes operand predicates, operand constraints, insn
conditions, the difference between define_insn vs define_expand and much
more.
On 2023/08/29 6:20, Jeff Law wrote:
What I would suggest is making a new predicate that accepts either a
register or a register+offset where the offset fits in a signed 12 bit
immediate. Use that for operand 0's predicate and I think this will
"just work" and cover all the cases supported by the prefetch.i instruction.
I misunderstood that as "just" adding the offset field to the
instructions and that's the reason I veered off the path so much. So
instead, I'll answer your original question.
register+offset seems a problem for prefetch instructions because signed
12 bit immediate values need to be also a multiple of 32. There's no
proper relocation type for this kind and I considered we have "very"
limited cases where making such predicate (as you suggested) will
*efficiently* work.
My opinion is, if we need very fine-grained control with prefetch
instructions, we'd better to use inline assembly.
I'll continue testing the possibilities of register+offset predicate
(including whether it works efficiently) and I'll temporarily withdraw
new built-in functions to focus on major issues before GCC 14:
1. Remove completely broken __builtin_riscv_zicbop_prefetch_i and
2. Fix an ICE when __builtin_prefetch is used with some constants.
I'll submit minimized patches only to fix those issues. They will not
contain "register+offset" you suggested because of the difficulties
above but should be sufficient to fix imminent issues.
We should be able to describe this need quite easily.
Each operand has a predicate which the compiler tests to see if a
particular RTL expression matches. Some are very generic like
"register_operand". Others are target specific. If you look in
predicates.md you'll see a list of the predicates already defined for
risc-v. I'm pretty sure none of them will work for this case, but we
can add a new one easily.
The operand in question is going to be a MEM with restrictions on its
addressing mode. Either REG or REG + aligned offset.
(define_predicate "prefetch_memory_operand"
(match_code "mem")
{
op = XEXP (op, 0);
return (REG_P (op)
|| (GET_CODE (op) == PLUS
&& REG_P (XEXP (op, 0))
&& CONST_INT_P (XEXP (op, 1))
&& (INTVAL (XEXP (op, 1)) % 32) == 0);
}
[ Note that we did not declare "op". It's provided by the generator and
corresponds to the operand we're testing. ]
So you're going to want a define_expand for the basic prefetch
(define_expand "riscv_prefetch_r_<mode>"
[(unspec_volatile:X [(match_operand:X 0 "memory_operand")]
UNSPEC_PREFETCH_R)]
"TARGET_ZICBOP"
{
if (!prefetch_memory_operand (Pmode, operands[0])
XEXP (operands[0], 0) = force_reg (Pmode, XEXP (operands[0], 0);
}
The thing to know about a define_expand is that it's sole purpose is for
RTL generation purposes. We can use it as a place to adjust operands
(as is done in this case), or emit additional RTL such as we do for
SImode max on rv64 where we have to extend the incoming operands.
In our case we see if the memory address matches
prefetch_memory_operand, and if not it'll force that address into a new
register to create a (mem (reg)) object.
(define_insn "*riscv_prefetch_r_<mode>"
[(unspec_volatile:X [(match_operand:X 0 "prefetch_memory_operand")]
UNSPEC_PREFETCH_R)]
"TARGET_ZICBOP"
"prefetch.r\t%0"
[(set_attr "type" "cbo")])
The define_insn construct maps an RTL template to assembly code with
provisions for testing operands and such.
Anyway, hopefully that makes things clearer.
Jeff
