On 12/7/23 09:04, Roger Sayle wrote:
Hi Jeff,
Doh! Great catch. The perils of not (yet) being able to actually
run any ARC execution tests myself.
ACK.
Shouldn't operands[4] be GEN_INT ((HOST_WIDE_INT_1U << tmp) - 1)?
Yes(-ish), operands[4] should be GEN_INT(HOST_WIDE_INT_1U << (tmp - 1)).
And the 32s in the test cases need to be 16s (the MSB of a five bit field is
16).
You're probably also thinking the same thing that I am... that it might be
possible
to implement this in the middle-end, but things are complicated by combine's
make_compound_operation/expand_compound_operation, and that
combine doesn't (normally) like turning two instructions into three.
Yea, I pondered, but didn't explore. I was expecting problems around
costing, make_compound_operation/expand_compound_operation didn't even
cross my mind, but I can certainly see how they'd be problematical.
Fingers-crossed the attached patch works better on the nightly testers.
The H8 variant without the -1 failed in my tester, but passed once the
-1 was added. I've got one thing to re-review as the -1 changes a few
things on the codegen side and I need to re-verify the profitability
across the various H8 configurations.
jeff
