[PATCH] D147835: [X86][mem-fold] Speed up test by not re-generating the .inc file

Fri Apr 7 23:46:55 PDT 2023

skan added a comment.

In D147835#4252780 <https://reviews.llvm.org/D147835#4252780>, @dblaikie wrote:

> Could you describe in more detail the purpose/value of this test? Judging by what I can see here it looks like it's a golden file test that verifies a copy of a file matches the current one - this doesn't seem valuable/seems like busy-work to keep an extra copy of a file around?

Sure, let me explain in more detail. `X86GenFoldTables.inc` consists of tables for memory folding and is used in some optimizations like RA, LICM, which is generated by a tblgen backend "-gen-x86-fold-tables". The .inc file is not in source code of LLVM and is produced in the build process. `x86-fold-tables.inc` is the only source file that contains the tables, so there is “extra copy" here.

We define some common rules in `X86FoldTablesEmitter.cpp` for the tblgen backend and these rules apply to most of existing and future instructions. You know, there are always exceptions and a future instruction may not comply with these rules, causing some entries in `X86GenFoldTables.inc` to be incorrect. We need a way a to detect it, this is the first reason this test exists.

Before D147527 <https://reviews.llvm.org/D147527> and we had this test, we expected developers to generate the `.inc` file in their local workspace and update hand-written tables biweekly or monthly. Unfortunately, people may forget it and tend to not fix the rules in `X86FoldTablesEmitter.cpp` when they were broken.  The longer we forget, the bigger the diff we need to review. The same wrong entries appeared in front of the reviewer again and again b/c no one fixed the broken rules.

If the developer can update the table or correct the broken rules each time a small part of instructions are added, the problem can be solved.  This is the second reason this test exists.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147835