[PATCH] D44176: [mips] Add support for CRC ASE.

[Simon Dardis via Phabricator via llvm-commits]

sdardis added inline comments.


================
Comment at: lib/Target/Mips/Mips32r6InstrInfo.td:949-955
+let AdditionalPredicates = [NotInMicroMips] in {
+  def CRC32B : R6MMR6Rel, CRC32B_ENC, CRC32B_DESC, ASE_CRC;
+  def CRC32H : R6MMR6Rel, CRC32H_ENC, CRC32H_DESC, ASE_CRC;
+  def CRC32W : R6MMR6Rel, CRC32W_ENC, CRC32W_DESC, ASE_CRC;
+  def CRC32CB : R6MMR6Rel, CRC32CB_ENC, CRC32CB_DESC, ASE_CRC;
+  def CRC32CH : R6MMR6Rel, CRC32CH_ENC, CRC32CH_DESC, ASE_CRC;
+  def CRC32CW : R6MMR6Rel, CRC32CW_ENC, CRC32CW_DESC, ASE_CRC;
----------------
vstefanovic wrote:
> sdardis wrote:
> > This needs ISA_MIPS32R6 for these instructions. The ASE_CRC adjective comes after the ISA adjective.
> This:
> ```
> def CRC32CW : R6MMR6Rel, CRC32CW_ENC, CRC32CW_DESC, ISA_MIPS32R6, ASE_CRC;
> ```
> ends up being the same as this:
> ```
> def CRC32CW : R6MMR6Rel, CRC32CW_ENC, CRC32CW_DESC, ASE_CRC;
> ```
> because (I think) ISA_MIPS32R6's InsnPredicates get overridden by ASE_CRC's InsnPredicates.
> We could do:
> ```
> class ASE_CRC {
>   list <Predicate> InsnPredicates = [HasCRC, HasMips32r6];
> }
> ```
> Btw, we might want to improve this with other ASEs too (in a separate patch), for example this MT case goes without a warning or error:
> ```
> .set mt
> .set mips2
> yield $4
> ```
> 
Good point. I have a posted patch which includes creating an additional field in PredicateControl which seperates ASEs from the other instruction predicates.

I'll factor it out of that patch and include some other refactoring I have in my local copy and commit that.

If we're getting no error from that assembly fragment you've posted, we can solve that in general and you can disregard my requested change for testing that .set correctly rejects the crc instructions.


Repository:
  rL LLVM

https://reviews.llvm.org/D44176