[PATCH] D159333: [RISCV] Replace RISCVVInversePseudosTable with a SearchIndex

[Craig Topper via Phabricator via llvm-commits]

craig.topper added a comment.

In D159333#4634476 <https://reviews.llvm.org/D159333#4634476>, @wangpc wrote:

> In D159333#4633456 <https://reviews.llvm.org/D159333#4633456>, @craig.topper wrote:
>
>> In D159333#4633394 <https://reviews.llvm.org/D159333#4633394>, @wangpc wrote:
>>
>>> In D159333#4633363 <https://reviews.llvm.org/D159333#4633363>, @craig.topper wrote:
>>>
>>>> In D159333#4633354 <https://reviews.llvm.org/D159333#4633354>, @wangpc wrote:
>>>>
>>>>> In D159333#4633304 <https://reviews.llvm.org/D159333#4633304>, @michaelmaitland wrote:
>>>>>
>>>>>> It looks like the InversePseudo table was only included for MCA directory. I wonder if this led to it only being a part of the llvm-mca binary, and not part of tools that do not depend on llvm-mca (such as llc). Could you check this by comparing the size of llc binary? It should have gotten larger after this change if it had not previously included the InversePseudo table.
>>>>>
>>>>> Yes, the size will increase about 100KB for llc.
>>>>> My intention is to unify the table (and make the code more consistent?). And if we can get SEW of pseudos from this table, then we don't need to specify the explict SEW imm in pseudos' operands (WIP).
>>>>> For example:
>>>>>
>>>>>   class VPseudoUSLoadNoMask<VReg RetClass,
>>>>>                             int EEW> :
>>>>>         Pseudo<(outs RetClass:$rd),
>>>>>                (ins RetClass:$dest, GPRMem:$rs1, AVL:$vl, ixlenimm:$sew,
>>>>>                     ixlenimm:$policy), []>
>>>>>
>>>>> becomes:
>>>>>
>>>>>   class VPseudoUSLoadNoMask<VReg RetClass,
>>>>>                             int EEW> :
>>>>>         Pseudo<(outs RetClass:$rd),
>>>>>                (ins RetClass:$dest, GPRMem:$rs1, AVL:$vl,
>>>>>                     ixlenimm:$policy), []>,
>>>>>
>>>>> This may simplify the generated patterns I think.
>>>>
>>>> Won't that require adding more pseudos for each SEW. Many pseudos don't have SEW today.
>>>
>>> I think almost all pseudos has `HasSEWOp` being true?
>>> **Edit:** I get it. I won't add more pseduos, but remove the SEW operand for existed pseudos.
>>
>> So there will 2 kinds of pseudos? Those with SEW in their name and those with an SEW operand?
>
> Yes.
> I just tried, if we remove the SEW operand of SEW-aware pseudos (load/store instructions are excluded in my experiment currently) operands and search SEW by RISCVVPseudosTable, we can reduce 30K of `llc`'s text section and about 6000 lines of `RISCVGenDAGISel.inc`. But it will also add some complexities and make the RVV pseudos inconsistent.
> I may post the patch after more investigations next week.

Where does the 30K text section reduction come from? What code are we reducing?

There should be a comment in RISCVGenDAGISel.inc that says "Total Array size is" can give the reduction on that value. That's more interesting than lines.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D159333