[llvm-commits] [PATCH] TableGen backend support to express relations between instruction

[Pranav Bhandarkar]

Hi Jyotsna,

My comments inline below.

>This patch adds TableGen support to create the relationship maps between
instructions. I have added a new class ‘InstrMapping’ in Target.td file to
model relations. New functionality in TableGen parses InstrMapping records
and uses the information to >generate relation map for the corresponding
relations. TableGen outputs it into  tables which are included at the end of
*GenInstrInfo.inc file. It also outputs functions to query these tables.


>+// InstrMapping - This class is used to create mapping tables to relate
>+// instructions with each other based on the values specified in
RowFields,
>+// ColFields, KeyCol and ValueCols.
>+//
>+class InstrMapping {
>+  // FilterClass - Used to limit search space only to the instructions
that
>+  // define the relationship modeled by this InstrMapping record.
>+  string FilterClass;
>+
>+  // RowFields - List of fields/attributes that should be same for all the
>+  // instructions in a row of the relation table. This is used
>+  // to categorize instructions in subgroups. For instance, if we want to
define
>+  // a relation that maps 'Add' instruction to its predicated forms, we
can
>+  // define RowFields like this:
>+  //
>+  // let RowFields = BaseOp
>+  // All add instruction predicated/non-predicated will have to set their
BaseOp
>+  // to the same value.
>+  //
>+  // def Add: { let BaseOp = 'ADD'; let predSense = 'nopred' }
>+  // def Add_predtrue: { let BaseOp = 'ADD'; let predSense = 'true' }
>+  // def Add_predfalse: { let BaseOp = 'ADD'; let predSense = 'false'  }
>+  list<string> RowFields = [];
>+
>+  // List of fields/attributes that are same for all the instructions
>+  // in a column of the relation table.
>+  // Ex: let ColFields = 'predSense' -- It means that the columns are
arranged
>+  // based on the 'predSense' values. All the instruction in a specific
>+  // column have the same value and it is fixed for the column according
>+  // to the values set in 'ValueCols'.
>+  list<string> ColFields = [];
>+
>+  // Values for the fields/attributes listed in 'ColFields'.
>+  // Ex: let KeyCol = 'nopred' -- It means that the key instruction
(instruction
>+  // that models this relation) should be non-predicated.
>+  // In the example above, 'Add' is the key instruction.
>+  list<string> KeyCol = [];
>+
>+  // List of values for the fields/attributes listed in 'ColFields', one
for
>+  // each column in the relation table.
>+  //
>+  // Ex: let ValueCols = [['true'],['false']] -- It adds two columns in
the
>+  // table. First column requires all the instructions to have predSense
>+  // set to 'true' and second column requires it to be 'false'.
>+  list<list<string> > ValueCols = [];
>+}
>+
>+//===---------------------------------------------------------------------
-===//

I think you should add comments to give a near-complete example of how a
particular instruction mapping would look For instance, if you could explain
how the predication mapping would look, that'll be really helpful.
Alternately, you could consider adding a section to the TableGen
documentation maintained on llvm.org. 

The reason I ask for a near-complete example is that it is not immediately
clear where things like "let ColFields = predSense" go. After thinking about
it for a bit, I am quite sure they will go into the def of the InstrMapping,
but an example would make it more obvious of course. Another thing, will
this table (predSense) have three columns (nopred, true & false) or only two
(true, false) ?


>+  // Emit function to performs binary search on relation tables.
>+  void emitBinSearchFunc(raw_ostream &OS);

<NitpickingStart> s/performs/perform/ <NitpickingEnd>

>+std::vector<Record*> MapTableEmitter::getKeyInstructions(InstrMap
&CurInstrMap,
>+                                             std::vector<Record*>
&InstrDefs) {

Indentation.


>+    std::vector<Record*> KeyInstrVec;
>+    KeyInstrVec = getKeyInstructions(CurInstrMap, InstrDefs);

Should you not assert if the size of KeyInstrVec is 0? Shouldn’t a mapping
have atleast one Key instruction ?

I have finished going through only a part of this patch. I'll post more
comments as I move along further.

Pranav
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by
The Linux Foundation