[PATCH] D122427: [MIPS] Initial support for MIPS-I load delay slots

[Spencer Alves via Phabricator via llvm-commits]

impiaaa marked 6 inline comments as done.
impiaaa added a comment.

In D122427#3407722 <https://reviews.llvm.org/D122427#3407722>, @djtodoro wrote:

> Since this is an initial support, I was wondering is there a clear set of TODOs that need to be done?

I don't actually know right now. I assumed that this is all that was needed for MIPS-I support. I only added the warning in because I hadn't tested on hardware. I'll try to find if there are any other differences between MIPS-I and MIPS-II.

In D122427#3409287 <https://reviews.llvm.org/D122427#3409287>, @sdardis wrote:

> I believe one issue that needs addressed is that the MipsAsmParser would need to be extended in the case of `.set reorder` being active, as loads would need to be padded with no-ops. Unfortunately when that directive is active nops need to be added after each load as there is no look-ahead.

Should that be a follow-up patch, or included in this one?



================
Comment at: llvm/lib/Target/Mips/MipsBranchExpansion.cpp:899-900
 
-  bool Changed =
-      longBranchChanged || forbiddenSlotChanged || fpuDelaySlotChanged;
+  bool Changed = longBranchChanged || forbiddenSlotChanged ||
+                 fpuDelaySlotChanged || loadDelaySlotChanged;
 
----------------
djtodoro wrote:
> is this clang-formatted?
Yes.


================
Comment at: llvm/test/CodeGen/Mips/mips1-load-delay.ll:1
+; RUN: llc < %s -mtriple=mips -mcpu=mips1 | FileCheck %s -check-prefixes=CHECK,MIPS1
+; RUN: llc < %s -mtriple=mips -mcpu=mips2 | FileCheck %s -check-prefixes=CHECK,MIPS2
----------------
sdardis wrote:
> You can use the script in llvm/utils/update_llc_test_checks.py to produce the test checks. This is somewhat easier that writing tests by hand.
> 
> Remove the 'CHECK' prefixes here, the script will complain if two RUN lines have the same prefixes. As there will only one CHECK prefix, you can change '-check-prefixes' to '-check-prefix'. 
> 
> You should also add a RUN line with '-mcpu=mips32r2' to add a case where the generate code is not MIPS-I or II.
Is there documentation somewhere on how to get the most out of update_llc_test_checks? I tried running it with this test and it generated assertions for 3 nearly-identical copies of all 28 lines of compiled code, which, in my opinion, is harder to read than assertions that call out the difference.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D122427/new/

https://reviews.llvm.org/D122427