[PATCH] D44687: [SchedModel] Remove instregex entries that don't match any instructions (WIP)

[Craig Topper via Phabricator via llvm-commits]

craig.topper added a comment.

This patch makes sure each regular expression covers at least one instruction. We already checked that each InstRW line matched at least one instruction. But if there were multiple regular expressions listed, we didn't check it.

Making sure each instruction is only covered by one instregex or instrs match is already being done. And a hole in it was fixed recently with r327808. Though I had to disable the improved check on some models. Look for FullInstRWOverlapCheck = 0 in the SchedMachineModel in the td files to see if I disabled it for any schedulers you care about. I need to collect the effected schedulers and start getting people to fix it.



================
Comment at: lib/Target/ARM/ARMScheduleR52.td:228
 def : InstRW<[R52WriteLd,R52Read_ISS],
-      (instregex "MOV_ga_pcrel_ldr", "t2MOV_ga_pcrel_ldr")>;
+      (instregex "MOV_ga_pcrel_ldr")>;
 
----------------
RKSimon wrote:
> javed.absar wrote:
> > Not sure why you removed 't2MOV_ga_pcrel_ldr' ? Is it repeated somewhere else ?
> Searching through ARMGenInstrInfo.inc I could see MOV_ga_pcrel_ldr but not t2MOV_ga_pcrel_ldr - that was what caused the error.
According to ARMGenInstrInfo.inc, there is no t2MOV_ga_pcrel_ldr


================
Comment at: lib/Target/ARM/ARMScheduleR52.td:287
+      "UMAAL", "t2SMLAL", "t2UMLAL",
       "t2SMLALBT", "t2SMLALTB", "t2SMLALTT", "t2SMLALD", "t2SMLALDX",
       "t2SMLSLD", "t2SMLSLDX", "t2UMAAL")>;
----------------
RKSimon wrote:
> javed.absar wrote:
> > t2MLALBB missing?
> Yup - doesn't exist in ARMGenInstrInfo.inc
t2MLALBB doesn't exist in ARMGenInstrInfo.inc


================
Comment at: utils/TableGen/CodeGenSchedule.cpp:152
+        PrintFatalError(Loc, "instregex has no matches: " + Original);
+#if 0 // TODO
+      if (1 == NumMatches)
----------------
javed.absar wrote:
> Just wondering why having 'just one match' would be a problem?
Having just one match means you should probably be using "(instrs" instead of "(instregex". The regular expression has to get tried on every instruction name that is defined. If there's ever only going to be one match that just wastes tablegen build time.


Repository:
  rL LLVM

https://reviews.llvm.org/D44687