Implemented *_DESC and *_ENC classes for MIPS64r6 LSA and DLSA instructions.
Details
Diff Detail
Event Timeline
LSA/DLSA in MIPS32r6/MIPS64r6 are the same instructions as in the MSA ASE. Can you explain the need for a second copy rather than correcting the predicates of the pre-existing one?
This patch implements support for LSA & DLSA instructions using predicates. This approach first includes all the MSA instructions in MIPS32r6/MIPS64r6 and then excludes all of them except LSA and DLSA. Maybe there is a better solution for this using predicates, but I didn't have a better idea. Zoran's suggestion is that we could reuse *_DESC and *_ENC classes from MSA, but to introduce new instruction definitions for MIPS32r6/MIPS64r6.
To be clear, I was only requesting an explanation for the seemingly redundant definition. I was guessing that the 32 feature bit limitation was likely to be the reason you couldn't follow the pattern used elsewhere (see below for details) in which case I would have requested a FIXME comment and approved it.
Your previous patch is better than the current one. The current one makes it impossible to use MSA on MIPS32r6/MIPS64r6.
See FeatureMips3_32 for an example of how to do a 'FeatureX || FeatureY' predicate. You define a feature that is the set of instructions in both MIPS32r6 and MSA (likewise for MIPS64r6 and MSA) and make that feature an implied bit for both MIPS32r6 and MSA. The snag is that this requires extra feature bits and tablegen currently asserts when there's more than 32. I have some plans for fixing this (either lifting the 32 feature limit, or adding support for an or condition) but it won't be a quick fix.
lib/Target/Mips/MipsMSAInstrInfo.td | ||
---|---|---|
3252 | It's a moot point since I've asked you to go back to the previous version of the patch but DLSA does not require the N64 ABI. It's requires 64-bit GPRs. |
You haven't answered the original question yet but I've just tried the FeatureMips3_32-style approach and we do hit the 32 feature bit limit. I know what needs to be done to fix that but I lack the time to do so at the moment. I therefore think it's best that we go ahead with this patch (with a minor change described below) and fix the 32 feature bit limit issue later.
Can you add a RUN line for mips32r6 and mips64r6 to test/CodeGen/Mips/msa/special.ll? E.g:
; RUN: llc -march=mips -mcpu=mips32r6 -mattr=+msa < %s | \ ; RUN: FileCheck %s --check-prefix=MIPS32 ; RUN: llc -march=mips64 -mcpu=mips64r6 -mattr=+msa < %s | \ ; RUN: FileCheck %s --check-prefix=MIPS64
This will ensure that the two definitions don't cause problems for MSA on MIPS32r6/MIPS64r6.
With that change it will LGTM.
It's a moot point since I've asked you to go back to the previous version of the patch but DLSA does not require the N64 ABI. It's requires 64-bit GPRs.