This is an archive of the discontinued LLVM Phabricator instance.

[X86][MemFold] Update some records for X86MemFoldTables.inc
ClosedPublic

Authored by yubing on Feb 2 2023, 12:49 AM.

Details

Diff Detail

Event Timeline

yubing created this revision.Feb 2 2023, 12:49 AM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 2 2023, 12:49 AM
yubing requested review of this revision.Feb 2 2023, 12:49 AM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 2 2023, 12:49 AM

sorry, i haven't run check-all. please wait, i may need to check testcases passrate

yubing added a comment.Feb 2 2023, 1:16 AM

sorry, i haven't run check-all. please wait, i may need to check testcases passrate

check-all fails are not related

yubing edited the summary of this revision. (Show Details)Feb 2 2023, 1:18 AM
skan added a comment.Feb 2 2023, 2:31 AM

Merge this patch into https://reviews.llvm.org/D142084 , then we can have a test coverage.

skan added a comment.Feb 24 2023, 8:02 PM

Generally LGTM, but could we remove the blank line at the end of the file?

Generally LGTM, but could we remove the blank line at the end of the file?

do we need to remove the blank line between each table, actually the blank line is generated by:

void printTable(const FoldTable &Table, StringRef TableName,
                formatted_raw_ostream &OS) {
  OS << "static const X86MemoryFoldTableEntry MemoryFold" << TableName
     << "[] = {\n";

  for (auto &E : Table)
    E.second.print(OS);

  OS << "};\n\n";
}

if we change OS << "};\n\n"; to OS << "};\n";
the blank line between each table will be lost as well.

skan accepted this revision.Mar 8 2023, 1:25 AM

Generally LGTM, but could we remove the blank line at the end of the file?

do we need to remove the blank line between each table, actually the blank line is generated by:

void printTable(const FoldTable &Table, StringRef TableName,
                formatted_raw_ostream &OS) {
  OS << "static const X86MemoryFoldTableEntry MemoryFold" << TableName
     << "[] = {\n";

  for (auto &E : Table)
    E.second.print(OS);

  OS << "};\n\n";
}

if we change OS << "};\n\n"; to OS << "};\n";
the blank line between each table will be lost as well.

I see. I thought it is an accidental blank

This revision is now accepted and ready to land.Mar 8 2023, 1:25 AM
This revision was landed with ongoing or failed builds.Mar 8 2023, 10:49 PM
This revision was automatically updated to reflect the committed changes.