[PATCH] D35625: Removal of microMIPS64R6

[Simon Dardis via Phabricator via llvm-commits]

sdardis added a comment.

I missed the AsmParser in my last reviews, can you add checks there, in the initialisation, in parseSetArchDirective and parseDirectiveSet?



================
Comment at: lib/Target/Mips/MipsSubtarget.cpp:108
+  if (hasMips64r6() && InMicroMipsMode) {
+    report_fatal_error("microMIPS64R6 is not supported");
+  }
----------------
Pass false as the second argument here, so that we don't emit a crash diagnostic.


================
Comment at: lib/Target/Mips/MipsTargetMachine.cpp:188-192
+  if (CPU == "mips64r6" && HasMicroMipsAttr) {
+    errs() << "LLVM currently does not support microMIPS for '" << CPU << "'.\n";
+    HasMicroMipsAttr = false;
+    HasNoMicroMipsAttr = true;
+  }
----------------
abeserminji wrote:
> sdardis wrote:
> > Take the context from the current Function and use emitError giving the function name + error message.
> > 
> > See include/llvm/IR/LLVMContext.h:300~ for emitError.
> I removed this check here, because I was not able to reproduce the error at this place. The check in //MipsSubtarget.cpp// at //107// always performs first, so this seemed to me like unnecessary thing. Although, correct me if I'm wrong, I might missed some case where this is called before the mentioned check in //MipsSubtarget.cpp//.
As far as I can see, this check can be dropped.


================
Comment at: test/CodeGen/Mips/micromips-64r6-disabled.ll:1
+; RUN: not llc -mtriple=mips64-unknown-linux -mcpu=mips64r6 -mattr=+micromips  %s 2>&1 | FileCheck %s
+
----------------
This file should be called micromips64r6-unsupported.ll.


================
Comment at: test/CodeGen/Mips/micromips-64r6-disabled.ll:3
+
+; test that error gets printed
+
----------------
Test that microMIPS64R6 is not supported.


Repository:
  rL LLVM

https://reviews.llvm.org/D35625