> It's worth noting there is a newer way which is usually slightly simpler > than a match_operator. Specifically code iterators.
Thank you for the very detailed feedback. It is not a problem to add code iterators. I would add iterators for "eq" and "ne" in riscv/iterators.md since they don't currently exist: > (define_code_iterator any_eq [eq ne]) I would also add new <optab> values for "eq" and "ne". I assume it would be best to submit the patch again as version 2 with these changes. > The pattern uses shifted_const_arith_operand, which is good as it > validates that the constant, if normalized by shifting away its trailing > zeros fits in a simm12. > > But the normalization you're doing on the two constants is limited by > the smaller of trailing zero counts. So operands2 might be 0x8100 which > requires an 8 bit shift for normalization. operands3 might be 0x81000 > which requires a 12 bit shift for normalization. In that case we'll use > 8 as our shift count for normalization, resulting in: > > 0x8100 >> 8 = 0x81, a valid small operand > 0x81000 >> 8 = 0x810, not a valid small operand. > > > I think that'll generate invalid RTL at split time. > > What I think you need to do is in the main predicate (the same place > you're currently !SMALL_OPERAND (INTVAL (operands[3]))), you'll need to > check that both operands are SMALL_OPERAND after normalization. Regarding the second issue, thanks again for the clear explanation. While at first this might seem like a problem, I believe these cases won't actually be a problem. The comparisons you mentioned, (x & 0x81000) == 0x8100 and (x & 0x8100) == 0x81000, will always evaluate as false, and this pattern will never be used for them (https://godbolt.org/z/Y11EGMb4f). Even in general, I'm quite sure we will never encounter an operand, after shifting, greater than 2^11 (i.e. not a SMALL_OPERAND). I will provide my reasoning below, but if you find it incorrect, or have any counterexamples, I would be happy to make the requested changes, add the mentioned check and submit that as PATCH v2. Lets consider the general expression (x & c1) == c2, where t1 and t2 represent the counts of trailing zeros in each constant. There are three cases to consider: 1. When t1 == t2: The pattern works fine, with no edge cases. 2. When t1 > t2: The expression will always evaluate as false, and the pattern won’t even be considered. For example, (x & 0x81000) == 0x8100. 3. When t1 < t2: In this case: - c1 must be of the form 0x0KLM00 (where the highest bit of K cannot be set) to meet the shifted_const_arith_operand constraint, ensuring SMALL_OPERAND(0x0KLM) == true (i.e. 0x0KLM < 2^11). - To prevent the expression from immediately evaluating as false, c2 must be in the form 0x0PQ<0bxxx0>00, where this value has to have only 0 or 1 in bit positions where c1 has 1 (and 0 elsewhere). Otherwise, (x & c1) == c2 would instantly be false, so this pattern wouldn’t be used. Lets call this "the critical condition". - After shifting c1 and c2, we have c1 == 0xKLM and c2 == 0xPQ<0bxxx0>, assuming the LSB of M is set to 1. - Due to "the critical condition", c2 == 0xPQ<0bxxx0> cannot have the highest bit of P set to 1. Otherwise, (x & c1) == c2 would immediately evaluate as false, since 0xKLM is guaranteed not to have the highest bit of K set to 1. This guarantees that SMALL_OPERAND(0xPQ<0bxxx0>) will always be true (i.e. c2 < 2^11). CONFIDENTIALITY: The contents of this e-mail are confidential and intended only for the above addressee(s). If you are not the intended recipient, or the person responsible for delivering it to the intended recipient, copying or delivering it to anyone else or using it in any unauthorized manner is prohibited and may be unlawful. If you receive this e-mail by mistake, please notify the sender and the systems administrator at straym...@rt-rk.com immediately.
