Page MenuHomePhabricator

[TableGen] Slim down the data structures in xxxGenInstrInfo.inc, step 1
ClosedPublic

Authored by Paul-C-Anagnostopoulos on Dec 14 2020, 6:14 AM.

Details

Summary

This simple patch reduces the size of the Offsets and OpcodeOperandTypes tables in the xxxGenInstrInfo.inc files. It is the first step in trying to reduce the size of data structures in those files.

Not until I made these changes did I realize that these two data structures are used only by the AVR target. But perhaps it will come into play in future targets.

Diff Detail

Unit TestsFailed

TimeTest
60 msx64 windows > LLVM.CodeGen/XCore::threads.ll
Script: -- : 'RUN: at line 1'; c:\ws\w16n2-1\llvm-project\premerge-checks\build\bin\llc.exe -march=xcore < C:\ws\w16n2-1\llvm-project\premerge-checks\llvm\test\CodeGen\XCore\threads.ll | c:\ws\w16n2-1\llvm-project\premerge-checks\build\bin\filecheck.exe C:\ws\w16n2-1\llvm-project\premerge-checks\llvm\test\CodeGen\XCore\threads.ll

Event Timeline

Paul-C-Anagnostopoulos requested review of this revision.Dec 14 2020, 6:14 AM
Paul-C-Anagnostopoulos created this revision.
Herald added a project: Restricted Project. · View Herald TranscriptDec 14 2020, 6:14 AM
craig.topper added a subscriber: craig.topper.EditedDec 14 2020, 5:51 PM

Does any in tree target use this function? I don't see any defines of GET_INSTRINFO_OPERAND_TYPE or calls to getOperandType. AVR only defines GET_INSTRINFO_OPERAND_TYPES_ENUM

lattner accepted this revision.Dec 14 2020, 9:20 PM

Looks reasonable to me, Paul mentions that only AVR is using it in tree.

This revision is now accepted and ready to land.Dec 14 2020, 9:20 PM

No, Craig is correct. AVR uses the GET_INSTRINFO_OPERAND_TYPES_ENUM data, but not the GET_INSTRINFO_OPERAND_TYPE data. My search was not specific enough.

Why do you suppose someone wrote this part of InstrInfoEmitter? Could it be used by an out-of-tree target? Could it once have been used but no longer?

I'd look at git blame, see who contributed it. We can then contact them, or just propose deletion of the code on llvm-dev and see if anyone speaks up :)

Here are the Phabricator discussions. Daniel Sanders says that he knows of out-of-tree users of these data structures.

https://reviews.llvm.org/D63320

https://reviews.llvm.org/rL366278

Here are the Phabricator discussions. Daniel Sanders says that he knows of out-of-tree users of these data structures.

https://reviews.llvm.org/D63320

https://reviews.llvm.org/rL366278

Yep, one bit I can share is that we use it to describe kinds of immediates. For example our MIRFormatter uses it to print named constants, bitfields, or plain immediates depending on this table.

@dsanders Which target is this?

And you're saying that you use the OpcodeOperandTypes table, not just the OperandType enum?

@dsanders Which target is this?

It's an out-of-tree proprietary target

And you're saying that you use the OpcodeOperandTypes table, not just the OperandType enum?

That's right, we use getOperandType() which uses that table

Okay then, I will clean up and then push this revision.