This is an archive of the discontinued LLVM Phabricator instance.

[mips] Refine octeon cins/cins32/exts/exts32 instruction.
Needs ReviewPublic

Authored by Kai on Jan 18 2015, 10:38 PM.
This revision needs review, but all reviewers have resigned.

Details

Reviewers
dsanders
Summary

The octeon CPU has special bit extraction and insertion instructions.
This patch adds a custom combiner to match these instructions.

I also tried to use a pattern, e.g. something like

let Predicates = [HasMips64, HasCnMips] in {
def : MipsPat<(shl (and i64:$lhs, BitMaskLO:$mask), uimm5_64:$pos),
              (CINS i64:$lhs, immZExt5_64:$pos, (BitMaskLen BitMaskLO:$mask))>;
}

but this will never match because the immediate of shl is always of type i32 (in the DAG).
In the pattern both operands must have the same type - i64 in this case.

Diff Detail

Event Timeline

Kai updated this revision to Diff 18369.Jan 18 2015, 10:38 PM
Kai retitled this revision from to [mips] Refine octeon cins/cins32/exts/exts32 instruction..
Kai updated this object.
Kai edited the test plan for this revision. (Show Details)
Kai updated this object.Jan 18 2015, 11:29 PM
Kai added a reviewer: dsanders.
Kai added a subscriber: Unknown Object (MLST).
Kai updated this revision to Diff 18403.Jan 19 2015, 2:13 PM
Kai updated this object.

Fixed a white space error, updated the tests

dsanders edited edge metadata.Jan 22 2015, 8:46 AM

I've got a couple comments.

lib/Target/Mips/Mips64InstrInfo.td
384–394

The two isCodeGenOnly's are to resolve the decoder conflict aren't they?

If you set 'DecoderNamespace = "Mips64"' on EXTS, EXTS32, CINS, and CINS32 then you'll resolve the decoder conflict and the disassembly will still disassemble all six cases.

lib/Target/Mips/MipsISelLowering.cpp
894–895

Else should be on the same line as the '}'

935–936

Personally I prefer to line-wrap before the '||' like this but LLVM code normally line-wraps after the '||' and it's best to be consistent.

950–951

Else should be on the same line as the '}'

lib/Target/Mips/MipsInstrInfo.td
130–133

I'd prefer to have two nodes (MipsCIns, and MipsExtS) and handle this CIns32/ExtS32 cases the same way you did BBIT0/BBIT032 (i.e. with a special ImmLeaf/PatLeaf and SDNodeXForm).

Kai updated this revision to Diff 26737.May 28 2015, 2:26 PM
Kai edited edge metadata.

Rebased to trunk and all code comments addressed. I still need to address the comment for the .td file.

dsanders resigned from this revision.Jul 12 2019, 4:11 PM