This is an archive of the discontinued LLVM Phabricator instance.

[mips] zeroext and logical 'and' mask optimizations
Needs RevisionPublic

Authored by sdardis on Jul 19 2016, 8:51 AM.

Details

Summary

This patch teaches the MIPS backend to use shifts and bitwise extract and
inserts when a solid mask of the form 0...1 or 1...0 rather than synthesising
the corresponding mask into a register and performing a logical and.

For MIPSR2, (d)ins(u) and (d)ext along with the zero register can be used to
implement masking operations that would be use logical and with a large
constant.

This reduces all such masking operations to a maximum of 2 instructions (shift
left+shift right or vice versa) or a minimum of one (bitwise extract or bitwise
insert with zero) in some cases.

As part of this, use the same trick to optimize zeroext for MIPS64R2.

Diff Detail

Event Timeline

sdardis updated this revision to Diff 64509.Jul 19 2016, 8:51 AM
sdardis retitled this revision from to [mips] zeroext and logical 'and' mask optimizations.
sdardis updated this object.
sdardis added reviewers: dsanders, vkalintiris.
sdardis set the repository for this revision to rL LLVM.
sdardis added a subscriber: llvm-commits.
dsanders accepted this revision.Jul 28 2016, 7:52 AM
dsanders edited edge metadata.

LGTM with some nits fixed

lib/Target/Mips/Mips64InstrInfo.td
600

indentation (align to the 'ZERO_64')

lib/Target/Mips/MipsInstrInfo.td
1093

I read this only being true for 0xffffffffffffffff but I know that's not what you're trying to say. It might be best to describe it in terms of leading ones and trailing zeros:

A mask where all bits are either leading ones or trailing zeros where there are at most 32 leading ones.
1096–1113

I think this could be simplified to:

bool Isi32 = N->getValueType(0) == MVT::i32;

uint64_t Value = N->getZExtValue();
if (Isi32)
  Value <<= 32;

unsigned TrailingZeros = countTrailingZeros(Value);
unsigned LeadingOnes = countLeadingOnes(Value);

return (LeadingOnes + TrailingZeros) == 64;

or alternatively:

if (N->getValueType(0) == MVT::i32) {
  uint32_t Value = N->getZExtValue();
  return countLeadingOnes(Value) + countTrailingZeros(Value) == 32;
}

uint64_t Value = N->getZExtValue();
return countLeadingOnes(Value) + countTrailingZeros(Value) == 64 && countLeadingOnes(Value) < 32;

Similarly for the others

1113

double semi-colon

2638–2645

This is for another patch, but we're not limited to leading or trailing ones. For example:

0x00ffff00

can be done with:

ext $rd, $rs, 8, 16

and:

0xff0000ff

can be done with:

ins $rd, $zero, 8, 16
2659

indentation (align to the 'RT')

2663

indentation (align to the 'RT')

test/CodeGen/Mips/fcopysign-f32-f64.ll
16–17

Why change the regex from {{[0-9]+}} to {{[0-9a-z]+}}? The register should always be numeric.

41–44

I believe the {{.*}} can only be 'f12'.

test/CodeGen/Mips/fcopysign.ll
9–17

This should be:

mfc1 $[[MFC:[0-9]+]], $f13
srl $[[R0:[0-9]+]], $[[MFC]], 32
11

I believe the {{.*}} can only be 'f12'. Likewise for the similar tests below.

test/CodeGen/Mips/llvm-ir/and-opts.ll
14

Could you handle the registers like fcopysign.ll does (i.e. with [[FOO:[0-9]+]] and [[FOO]])? This is important for the multi-instruction cases below.

Also, why is [a-z] permitted in the register numbers?

24

The '20' is missing on this line

37

The '24' is missing on this line

83

I believe the second operand must be $zero. Likewise for the other dinsu's below

This revision is now accepted and ready to land.Jul 28 2016, 7:52 AM
sdardis updated this revision to Diff 66638.Aug 3 2016, 3:56 AM
sdardis edited edge metadata.
sdardis removed rL LLVM as the repository for this revision.
sdardis marked 3 inline comments as done.

Would you mind taking a second look at this?

Addressed review comments, added DEXT64_32_MM64R6 as llvm-mc would match dext in microMIPS64R6 to MIPS64R2's dext which has a different encoding.

sdardis marked 3 inline comments as done.Aug 3 2016, 4:00 AM
sdardis added inline comments.
lib/Target/Mips/MipsInstrInfo.td
2638–2645

I was considering that but judged to a bit too specific do do in the first round of this potential patch series. I mostly wanted to hit the bad cases of zext.

vkalintiris edited edge metadata.Sep 20 2016, 6:31 AM

Can you take a look at CodeGen/Mips/cconv/arguments-varargs.ll because it fails for me (and rebase the patch while at it :))?

vkalintiris requested changes to this revision.Oct 18 2016, 4:58 AM
vkalintiris edited edge metadata.

Marking this with "Request Changes" based on my last comment.

This revision now requires changes to proceed.Oct 18 2016, 4:58 AM
dsanders resigned from this revision.Jul 18 2019, 7:03 PM