[llvm-commits] [PATCH] Refactor llvm-mc to improve -disassemble

[Richard Barton]

Hi Ben

Thanks for the review. I have committed the patch as r154809.

Ta
Rich

> -----Original Message-----
> From: Benjamin Kramer [mailto:benny.kra at googlemail.com]
> Sent: 15 April 2012 22:25
> To: Richard Barton
> Cc: llvm-commits at cs.uiuc.edu
> Subject: Re: [llvm-commits] [PATCH] Refactor llvm-mc to improve -disassemble
>
>
> On 04.04.2012, at 20:38, Richard Barton wrote:
>
> > Hello Reviewers
> >
> > The attached patch is a refactoring of llvm-mc.
> >
> > The initial motivation was to allow --show-inst and --show-encoding to work
> for
> > -disassemble in the same way as for -assemble. The solution was to use an
> > MCAsmStreamer to produce the output for -disassemble, rather than use the
> > InstPrinter directly.
> >
> > As part of this change I have moved all the duplicated creation of MC
> Objects
> > into the main function.
> >
> > There was one problem with this refactoring. -assemble setup the InstPrinter
> so
> > that the assembly syntax variant was altered by the command line argument
> > --output-asm-syntax. -disassemble ignored this argument altogether and
> always
> > used MCAsmInfo::getAssemblerDialect to set this.
> >
> > I propose that -disassemble change to use the command line option like the
> > -assemble. This causes one X86 disassembly test to fail as it relied on the
> old
> > behaviour. The second patch changes the test to specify the correct assembly
> > syntax in the new way.
> >
> > The alternative would be to switch the default assembly syntax behaviour
> based
> > on whether disassemble or assemble is chosen. I don't think this is very
> nice,
> > but I'm happy to change the patch if people want.
> >
> > I have split the patch down:
> >
> > Patch 01: Refactoring - move all the MC Object creation into main
> > Patch 02: Test Change to make X86 disassemble test pass
> > Patch 03: Have -disassemble use an AsmStreamer instead of an InstPrinter
>
> Patches look good to me! Do you need someone to commit it for you?
>
> - Ben
>
> >
> > Please review.
> >
> > Regards,
> >
> > Richard Barton
> >
> <01_llvm_mc_refactor.patch><02_llvm_mc_test_change.patch><03_llvm_mc_show_inst
> _encoding.patch><llvm_mc_all.patch>___________________________________________
> ____
> > llvm-commits mailing list
> > llvm-commits at cs.uiuc.edu
> > http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>


-- IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium.  Thank you.