[PATCH] D89114: [TableGen][SchedModels] Fix aliasing of SchedWriteVariant

[Dave Green via Phabricator via llvm-commits]

dmgreen added a comment.

I don't know this tablegen code very well, but I've read it through and it seems to make sense. I agree that the results look better in line with how the schedule model was written.



================
Comment at: llvm/test/TableGen/sched-aliases.td:1
+// REQUIRES: aarch64-registered-target
+// RUN: llvm-tblgen -gen-instr-info %s -I%p/../../include -I%p/../../lib/Target/AArch64 -o %t -debug-only=subtarget-emitter 2>&1 | FileCheck %s 
----------------
You may need REQUIRES: asserts too, to get dbg output.


================
Comment at: llvm/test/TableGen/sched-aliases.td:47
+
+def ProcFoo0 : SubtargetFeature<"foo-0", "ARMProcFamily", "foo-0",
+                                "Test Processor #1", []>;
----------------
Is ProcFoo0 needed?


================
Comment at: llvm/test/tools/llvm-mca/ARM/A57-sxtb.s:2
+# NOTE: Assertions have been autogenerated by utils/update_mca_test_checks.py
+# RUN: llvm-mca -mtriple=armv7 -mcpu=cortex-a57 -instruction-tables < %s | FileCheck %s
+
----------------
armv7 -> armv8

Is this file generated with the script? I would expect more tables at the bottom.

Can you pre-commit the file and show the differences here.


================
Comment at: llvm/utils/TableGen/CodeGenSchedule.cpp:1678
+    // we also have InstRWs.
+    SCTrans.ProcIndices.erase(
+        llvm::remove_if(SCTrans.ProcIndices,
----------------
Could we just not add them to begin with?


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

https://reviews.llvm.org/D89114