On 7/1/24 10:48, Paolo Bonzini wrote:
On Mon, Jul 1, 2024 at 4:51 AM Richard Henderson
<richard.hender...@linaro.org> wrote:
While debugging #2413, I spent quite a bit of time trying to work
out if the CCOp value was incorrect. I think the following is a
worthwhile cleanup, isolating potential problems to asserts.
Hi Richard,
no objections at all to introducing more asserts. I think keeping the
array is a better underlying implementation for cc_op_live() however.
Hmm. I had an implementation that would detect missing entries at runtime, but this one
detects missing entries at compile time.
I'm also not very fond of mixing "sized" and "unsized" CCOps in the
4..7 range, there's no real reason why CC_OP_DYNAMIC and CC_OP_CLR
must be close to CC_OP_EFLAGS and the ADCOX CCOps. I also think it's
clearer to keep CC_OP_POPCNT[BWLQ] (even though in practice only one
will be used because popcnt needs zero extension anyway).
My objection to keeping the unused POPCNT* enumerators is that it interferes with proper
cooperation with -Wswitch, to diagnose missing enumerators. This is also why I removed
CC_OP_NB.
If those extra enumerators are present, you either have to include them explicitly in the
switch, which is less than desirable, or add a default case, which disables -Wswitch entirely.
If you don't like abusing 4..7, then we can use 8..11, placing POPCNT at 10 or
11.
As an aside, I'm wondering if CC_OP_CLR is particularly important; I
expect "xor reg, reg" to be followed by more ALU operations most of
the time and to not be followed by a jump, so it only saves a spill if
xor reg, reg is followed by a lot or store. If gen_XOR used either
CC_OP_LOGICn or CC_OP_EFLAGS for "xor reg, reg", the values in
decode->cc_op_* (CC_OP_DST=0 for CC_OP_LOGICn; CC_OP_SRC=CC_Z|CC_P for
CC_OP_EFLAGS) would be constant and wouldn't add to register pressure.
You could easily be right. Improvements to tcg in the last 11 years may have made it
redundant, or it might have been wishful thinking even at the time.
r~
