[PATCH] TableGen minor refactoring: AsmWriterEmitter and CodeGenDAGPatterns

Ahmed Bougacha ahmed.bougacha at gmail.com
Mon Oct 14 18:00:40 PDT 2013


On Tue, Oct 15, 2013 at 2:33 AM, Quentin Colombet <qcolombet at apple.com> wrote:
> Hi Ahmed,
>
> Good to see you back on the mailing list!
>
> I have looked only at the first patch at the moment.
>
> Here are my remarks:
> * I do not understand this comment.
> -/// implementation.
> +/// implementation. Destroys AsmWriterInst information.
>
> You mean that the information hold in AsmWriterEmitter::Instructions is
> destroyed, right?

Yes, the meaning was: it destroys all instances of AsmWriterInst
(stored in AsmWriterEmitter::Instructions); I'll rephrase that.

> * What is the point of this refactoring?
> I completely missed the point of this first refactoring.
> You avoid the cost of building a CodeGenTarget several times, which is nice,
> but I do not see the benefits for the vector of AsmWriterInst.
> This does not seems useful either for futur reference, as the field is
> private and there is no accessors.

Two reasons:

- first, the CGIAWIMap kept pointers to the elements of
AsmWriterEmitter::Instructions. The former is a member of
AsmWriterEmitter, the latter is local to
AsmWriterEmitter::EmitPrintInstruction, so there's a lifetime problem
there. This isn't a big deal, but isn't great either.

- second, this enables another patch I have, which adds a method to
AsmWriterEmitter that generates a generic printOperand method, based
on the print methods that were used: this is where the
AsmWriterEmitter::Instructions vector is useful.

Thanks for reviewing, Quentin!

- Ahmed

> Thanks,
> -Quentin
>
> On Oct 9, 2013, at 12:56 PM, Ahmed Bougacha <ahmed.bougacha at gmail.com>
> wrote:
>
> Hello everyone,
>
> Here are 2 patches that prepare for upcoming additions. There
> shouldn't be any functionality change.
> Thanks for reviewing!
>
> - Ahmed
>
> ----
>
>    TableGen: Refactor DAG patterns to enable parsing one pattern at a time.
>
> utils/TableGen/CodeGenDAGPatterns.cpp | 112
> utils/TableGen/CodeGenDAGPatterns.h   |   5
> 2 files changed, 66 insertions(+), 51 deletions(-)
>
> ----
>
>    TableGen: Refactor AsmWriterEmitter to keep AsmWriterInsts.
>
>    These used to be referenced by the CGI->AWI map (in AsmWriterEmitter),
> but
>    stored in a vector local to EmitPrintInstruction. Move the vector to
>    AsmWriterEmitter too.
>
> utils/TableGen/AsmWriterEmitter.cpp | 51
> 1 file changed, 25 insertions(+), 26 deletions(-)
> <0001-TableGen-Refactor-AsmWriterEmitter-to-keep-AsmWriter.patch><0002-TableGen-Refactor-DAG-patterns-to-enable-parsing-one.patch>_______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
>



More information about the llvm-commits mailing list