On 8/27/24 08:19, Jeff Law wrote:
On 8/26/24 6:37 PM, Patrick O'Neill wrote:
This patch adds some advanced checking to assert that the emitted
costs match
emitted patterns for const_vecs.
Flow:
Costing: Insert into hashmap<rtx, vec<(const_rtx, enum)>>
Expand: Check for membership in hashmap
-> Not in hashmap: ignore, this wasn't costed
-> In hashmap: Iterate over vec
-> if RTX not in hashmap: Ignore, this wasn't costed (hash
collision)
-> if RTX in hashmap: Assert enum is expected
There are no false positive asserts with this flow.
gcc/ChangeLog:
* config/riscv/riscv-v.cc (expand_const_vector): Add RTL_CHECKING
gated
asserts.
* config/riscv/riscv.cc (riscv_const_insns): Ditto.
* config/riscv/riscv-v.h (insert_expected_pattern): Add helper
function
to insert hash collisions into hash map vec key.
(get_expected_costed_type): Add helper function to get the expected
cost type for a given rtx pattern.
I suspect this is going to be problematical at some point,
particularly since we can get hash conflicts for cases that aren't
problematical.
The concern here is that the hash -> vec mapping will end up containing
too many entries in the vec due to hash collisions?
In general we also want to avoid #ifdefs -- we're not clean in that
regards by any means, but much of that cruft has been converted to a
runtime check. The basic idea is that conditionally compiled code
like that tends to be problematical for various checks like unused
variables/paramters, use-without-defintion objects, etc.
I'd tend to prefer to drop this, but I'm not steadfastly against
including.
Makes sense - the more I've thought about it the less happy I am with
it's implementation. I'll put it on the backburner for now and see if
there's a more elegant solution I can poke at.
Thanks for the review!
Patrick
