Hi Segher,
It's great to hear from you again. It's been a while.
>On Fri, Jun 19, 2020 at 09:42:54PM +0100, Roger Sayle wrote:
>> My recent patch to add scalar integer simplification unit tests to
>> simplify_rtx_c_tests identified two "trivial" corner cases that could be
improved in simplify-rtx.c.
>
> These two things are independent changes and should be independent
patches.
I'm about to ask you (very nicely) to please commit these changes for me,
so many thanks in advance if you're going to be making twice the effort.
>> Although it makes no sense to ever see a BImode ROTATE, the current
>> ordering of transformations in simplify_binary_operation_1 converts
>> (rotate:bi (reg:bi) 0) to (rotatert:bi (reg:bi) 1), which then doesn't
>> get simplified away. Rather than teach the middle-end that any
>> hypothetical ROTATE or ROTATERT of a BImode value is a no-op, a more
realistic invariant is that any rotate by const0_rtx is already canonical.
>
> I don't know of a rotate of BImode is defined at all, but that is beside
the point, the patch has nothing to do with BImode.
> A rotate by constant 0 should be simplified to just its argument, and that
should happen *before* where your patch makes changes. What goes wrong
there?
This approach becomes clearer from seeing the larger context of this change:
case ROTATERT:
case ROTATE:
/* Canonicalize rotates by constant amount. If op1 is bitsize / 2,
prefer left rotation, if op1 is from bitsize / 2 + 1 to
bitsize - 1, use other direction of rotate with 1 .. bitsize / 2 -
1
amount instead. */
#if defined(HAVE_rotate) && defined(HAVE_rotatert)
if (CONST_INT_P (trueop1)
&& IN_RANGE (INTVAL (trueop1),
GET_MODE_UNIT_PRECISION (mode) / 2 + (code ==
ROTATE),
GET_MODE_UNIT_PRECISION (mode) - 1))
{
int new_amount = GET_MODE_UNIT_PRECISION (mode) - INTVAL
(trueop1);
rtx new_amount_rtx = gen_int_shift_amount (mode, new_amount);
return simplify_gen_binary (code == ROTATE ? ROTATERT : ROTATE,
mode, op0, new_amount_rtx);
}
#endif
/* FALLTHRU */
case ASHIFTRT:
if (trueop1 == CONST0_RTX (mode))
return op0;
Hence the handling of rotates and shifts by zero is the very next
optimization, after the fall-thru.
One option would be to duplicate this const0 optimization before the rotate
canonicalization, then have
it tested a second time later (perhaps twice sequentially if the backend
doesn't support both rotates)
or add a goto. I believed the simplest fix is to correct the condition for
canonicalizing which shouldn't
fire when op1 is const0_rtx.
You'll also notice that by placing "trueop1 != CONST0_RTX (mode)" condition
as the very last clause
in the test, this creates a jump threading opportunity for the compiler.
[p.s. Many thanks again
for your help back in 2002/2003 when I originally added jump threading to
GCC].
>> Optimizing "parity of parity" matches the tree constant folding
>> transformation pending review. Alas, the only mentions of PARITY in
>> GCC's official backend machine descriptions are in expanders, so the
>> middle-end's RTL optimizers never see a PARITY to simplify.
>
> This part is approved for trunk. Thanks!
Thanks to you too. Alas, my credentials from the CVS days of GCC almost
certainly don't
work any more (in git), so I'm now reliant on the generosity of others to
push changes
into the tree. I promise that if I make a habit of contributing regularly
again, I'll learn
the new ways of doing things, but it's a little awkward after being a
maintainer.
>> A test can be added to test_scalar_int_ops once that patch is
reviewed/approved.
> If you insist. I find this futile busy-work, now and in the future :-/
I'm just following Richard Sandiford suggestions, but I will concede that
these internal
tests did spot the (admittedly minor) logic issue above.
Please let me know what you think. And many thanks again.
Cheers,
Roger
--
